The following reply was made to PR kern/36673; it has been noted by GNATS.

From: Juan RP <juan@xtrarom.org>
To: yamt@mwd.biglobe.ne.jp (YAMAMOTO Takashi)
Cc: gnats-bugs@NetBSD.org
Subject: Re: kern/36673: dubious code in sysmon_envsys
Date: Sat, 21 Jul 2007 18:03:43 +0200

 On Sun, 22 Jul 2007 00:45:02 +0900 (JST)
 yamt@mwd.biglobe.ne.jp (YAMAMOTO Takashi) wrote:
 
 > because it's the simplest way to handle errors.
 > and it's clearer, at least than leaving partially initialized data.
 
 Ok, I think you are right. I think we should have at least three or
 four required objects in a dictionary to be created:
 
 	- state
 	- type
 	- description
 	- something else?
 
 > because it isn't documented.  it might be clear to you, but not to me.
 
 Do I have to explain how the locking works in the code? I think that if
 you see the code, you can see how it works.
 
 There is source less documented than mine and I don't complain because
 there's not stuff documented...
  
 > because i don't understand why you want to modify sme_nsensors here.
 > 
 > besides, the current code:
 > 
 >         for (i = 0; i < sme->sme_nsensors; i++) {
 >                 edata = &sme->sme_sensor_data[i];
 >                 if (edata->flags & ENVSYS_FDUPDESC) {
 >                         --sme->sme_nsensors;
 >                         --i;
 >                         continue;
 >                 }
 > 
 > "--i; continue;" means "i" will be the same value in the next
 > iteration of the loop.  so it will look at the same "edata", and the
 > ENVSYS_FDUPDESC condition will always to be true until the loop exits.
 > so it's effectively same as:
 > 
 >         for (i = 0; i < sme->sme_nsensors; i++) {
 >                 edata = &sme->sme_sensor_data[i];
 >                 if (edata->flags & ENVSYS_FDUPDESC) {
 > 			sme->sme_nsensors = i;
 > 			break;
 >                 }
 > 
 > is it what you intend?
 
 But that would mean that if you break and it's not the last sensor,
 next sensors will be skipped... right? this is not what we want to make
 it show all valid sensors.
 
 > suppose that sme->sme_nsensors == 2.
 > if an error happens for sme->sme_sensor_data[0], it will not be
 > returned to the caller of sysmon_envsys_createplist.  (unless it was
 > EINVAL) otoh, if an error happens for sme->sme_sensor_data[1], it
 > will be returned to the caller of sysmon_envsys_createplist.
 
 You are right. I changed the code to check for all errors, let me know
 if there are more problems.
 
 -- 
 Juan Romero Pardines 	- The NetBSD Project
 http://plog.xtrarom.org 	- NetBSD/pkgsrc news in Spanish