Subject: Re: kern/36673: dubious code in sysmon_envsys
To: None <kern-bug-people@netbsd.org, gnats-admin@netbsd.org,>
From: Juan RP <juan@xtrarom.org>
List: netbsd-bugs
Date: 07/21/2007 16:20:02
The following reply was made to PR kern/36673; it has been noted by GNATS.
From: Juan RP <juan@xtrarom.org>
To: gnats-bugs@NetBSD.org
Cc: yamt@mwd.biglobe.ne.jp
Subject: Re: kern/36673: dubious code in sysmon_envsys
Date: Sat, 21 Jul 2007 18:19:35 +0200
On Sat, 21 Jul 2007 16:05:03 +0000 (UTC)
Juan RP <juan@xtrarom.org> wrote:
> > 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.
I removed this bogus '--i' and all works as expected. I think this was
required in a previous implementation that I had, but not needed since
I added a SLIST to handle the sensor descriptions.
> > 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.
Thanks. I think there's only one point to fix your PR right now?
--
Juan Romero Pardines - The NetBSD Project
http://plog.xtrarom.org - NetBSD/pkgsrc news in Spanish