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

From: yamt@mwd.biglobe.ne.jp (YAMAMOTO Takashi)
To: juan@xtrarom.org
Cc: gnats-bugs@NetBSD.org
Subject: Re: kern/36673: dubious code in sysmon_envsys
Date: Sun, 22 Jul 2007 01:26:03 +0900 (JST)

 > > 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...
 
 if you want someone understand (and possibly fix) your code,
 it's better to explain unless it's obvious.
 
 to me, the locking here seems far from obvious.  for example, to me,
 
 - it isn't clear which of sme_mtx or sme_list_mtx protects sysmon_envsys_list.
 
 - it isn't clear if it's safe to drop sme_mtx in the middle of
  sme_make_dictionary.
 
 > > 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.
 
 although i'm not sure what you want to do here,
 a usual way to "skip" an entry in a loop is just "continue".
 
 	for (i = 0; i < sme->sme_nsensors; i++) {
 		edata = &sme->sme_sensor_data[i];
 		if (edata->flags & ENVSYS_FDUPDESC) {
 			continue;
 		}
 
 can you explain why you want to modify sme_nsensors here?
 it isn't obvious to me.
 
 YAMAMOTO Takashi