Subject: Re: bin/34152: envstat(8) can't find some sensors when there is a hole in sensor numbers
To: None <gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, bsh@netbsd.org>
From: Hiroyuki Bessho <bsh@grotto.jp>
List: netbsd-bugs
Date: 08/07/2006 07:50:02
The following reply was made to PR bin/34152; it has been noted by GNATS.

From: Hiroyuki Bessho <bsh@grotto.jp>
To: gnats-bugs@netbsd.org
Cc: bsh@netbsd.org
Subject: Re: bin/34152: envstat(8) can't find some sensors when there is a hole in sensor numbers
Date: Mon, 07 Aug 2006 16:47:23 +0900

   The patch I sent was not perfect. This one is better.
 
 Index: envstat.c
 ===================================================================
 RCS file: /remote/kamasu/u0/cvsupbase/cvsroot-netbsd/src/usr.sbin/envstat/envstat.c,v
 retrieving revision 1.22
 diff -u -u -r1.22 envstat.c
 --- envstat.c	3 Jun 2004 16:48:53 -0000	1.22
 +++ envstat.c	7 Aug 2006 07:37:43 -0000
 @@ -55,7 +55,7 @@
  
  int main(int, char **);
  void listsensors(envsys_basic_info_t *, size_t);
 -size_t numsensors(int);
 +int findsensors(int, envsys_tre_data_t *, envsys_basic_info_t *, int, int);
  void fillsensors(int, envsys_tre_data_t *, envsys_basic_info_t *, size_t);
  size_t longestname(envsys_basic_info_t *, size_t);
  int marksensors(envsys_basic_info_t *, int *, char *, size_t);
 @@ -79,6 +79,7 @@
  	int *cetds;
  	char *sensors;
  	const char *dev;
 +	struct envsys_range rng;
  
  	fd = -1;
  	ls = 0;
 @@ -133,9 +134,14 @@
  	 * Determine number of sensors, allocate and fill arrays with
  	 * initial information.  Determine column width
  	 */
 -	if ((ns = numsensors(fd)) == 0)
 +	rng.units = -1;			/* All type of sensors */
 +	if (ioctl(fd, ENVSYS_GRANGE, &rng))
 +		err(1, "ioctl(ENVSYS_GRANGE) failed");
 +
 +	if (rng.low > rng.high)
  		errx(1, "No sensors found");
  
 +	ns = rng.high - rng.low + 1;
  	cetds = (int *)malloc(ns * sizeof(int));
  	etds = (envsys_tre_data_t *)malloc(ns * sizeof(envsys_tre_data_t));
  	ebis = (envsys_basic_info_t *)malloc(ns * sizeof(envsys_basic_info_t));
 @@ -143,7 +149,10 @@
  	if ((cetds == NULL) || (etds == NULL) || (ebis == NULL))
  		err(1, "Out of memory");
  
 -	fillsensors(fd, etds, ebis, ns);
 +	ns = findsensors(fd, etds, ebis, rng.low, rng.high);
 +
 +	if (ns <= 0)
 +		errx(1, "No sensors found");
  
  	if (ls) {
  		listsensors(ebis, ns);
 @@ -373,51 +382,51 @@
  
  /*
   * pre:  fd contains a valid file descriptor of an envsys(4) supporting device
 - * post: returns the number of valid sensors provided by the device
 - *       or -1 on error
 + *       && ns is the number of sensors
 + *       && etds and ebis are arrays of sufficient size
 + * post: return number of valid sensors.
   */
 -size_t
 -numsensors(int fd)
 +int
 +findsensors(int fd, envsys_tre_data_t *etds, envsys_basic_info_t *ebis,
 +    int low, int high)
  {
 -	int count = 0, valid = 1;
 -	envsys_tre_data_t etd;
 -	etd.sensor = 0;
 +	int i, n = 0;
  
 -	while (valid) {
 -		if (ioctl(fd, ENVSYS_GTREDATA, &etd) == -1)
 -			err(1, "Can't get sensor data");
 +	for (i=low; i <=high; ++i) {
 +		ebis[n].sensor = i;
 +		if (ioctl(fd, ENVSYS_GTREINFO, &ebis[n]) == -1)
 +			err(1, "Can't get sensor info for sensor %d", i);
  
 -		valid = etd.validflags & ENVSYS_FVALID;
 -		if (valid)
 -			++count;
 +		etds[n].sensor = i;
 +		if (ioctl(fd, ENVSYS_GTREDATA, &etds[n]) == -1)
 +			err(1, "Can't get sensor data for sensor %d", i);
  
 -		++etd.sensor;
 +		if ((ebis[n].validflags & ENVSYS_FVALID) &&
 +		    (etds[n].validflags & ENVSYS_FVALID) )
 +			++n;
  	}
  
 -	return count;
 +	return n;
 +	
  }
  
  /*
   * pre:  fd contains a valid file descriptor of an envsys(4) supporting device
   *       && ns is the number of sensors
   *       && etds and ebis are arrays of sufficient size
 - * post: returns 0 and etds and ebis arrays are filled with sensor info
 - *       or returns -1 on failure
   */
  void
 -fillsensors(int fd, envsys_tre_data_t *etds, envsys_basic_info_t *ebis,
 -    size_t ns)
 +fillsensors(int fd, envsys_tre_data_t *etds, envsys_basic_info_t *ebis, size_t ns)
  {
  	int i;
  
 -	for (i = 0; i < ns; ++i) {
 -		ebis[i].sensor = i;
 +	for (i=0; i < ns; ++i) {
  		if (ioctl(fd, ENVSYS_GTREINFO, &ebis[i]) == -1)
  			err(1, "Can't get sensor info for sensor %d", i);
  
 -		etds[i].sensor = i;
  		if (ioctl(fd, ENVSYS_GTREDATA, &etds[i]) == -1)
  			err(1, "Can't get sensor data for sensor %d", i);
 +
  	}
  }
  
 @@ -450,6 +459,7 @@
  {
  	size_t i;
  	char *s;
 +	int found=0;
  
  	if (sensors == NULL) {
  		/* No sensors specified, include them all */
 @@ -465,8 +475,10 @@
  	s = strtok(sensors, ",");
  	while (s != NULL) {
  		int snum;
 -		if ((snum = strtosnum(ebis, s, ns)) != -1)
 +		if ((snum = strtosnum(ebis, s, ns)) != -1) {
  			cetds[snum] = 1;
 +			found=1;
 +		}
  		else {
  			warnx("Unknown sensor %s", s);
  			return (-1);
 @@ -476,9 +488,8 @@
  	}
  
  	/* Check if we have at least one sensor selected for output */
 -	for (i = 0; i < ns; ++i)
 -		if (cetds[i] == 1)
 -			return (0);
 +	if (found)
 +		return (0);
  
  	warnx("No sensors selected for display");
  	return (-1);
 @@ -497,7 +508,7 @@
  	for (i = 0; i < ns; ++i) {
  		if((ebis[i].validflags & ENVSYS_FVALID) &&
  		   !strcmp(s, ebis[i].desc))
 -			return ebis[i].sensor;
 +			return i;
  	}
  
  	return -1;