Subject: Re: lib/35403: Error code path optimization in libc's implementation of uname()
To: None <lib-bug-people@netbsd.org, gnats-admin@netbsd.org,>
From: Christian Biere <christianbiere@gmx.de>
List: netbsd-bugs
Date: 01/15/2007 20:55:01
The following reply was made to PR lib/35403; it has been noted by GNATS.

From: Christian Biere <christianbiere@gmx.de>
To: gnats-bugs@NetBSD.org, netbsd-bugs@netbsd.org
Cc: 
Subject: Re: lib/35403: Error code path optimization in libc's implementation of uname()
Date: Mon, 15 Jan 2007 22:00:00 +0100

 Martin Husemann wrote:
 > On Thu, Jan 11, 2007 at 01:50:00AM +0000, Pierre Pronchery wrote:
 > >  It would be even more efficient to directly return in case of
 > > errors in this function, since it can make up to four useless other
 > > calls to sysctl() in this situation.
  
 > I could see a point doing this for the first sysctl - but if we touch the
 > utsname argument, we should not leave it half-initialized, IMHO.
 
 How about the compromise below? I avoided memset() because struct utsname
 is rather big.
  
 > I can not see any reason for this sysctl to fail though, so this change
 > hardly matters.
 
 Yes, it's likely not worth it. Commit or just close?
 
 -- 
 Christian
 
 
 Index: uname.c
 ===================================================================
 RCS file: /cvsroot/src/lib/libc/gen/uname.c,v
 retrieving revision 1.9
 diff -u -p -r1.9 uname.c
 --- uname.c	7 Aug 2003 16:42:59 -0000	1.9
 +++ uname.c	15 Jan 2007 20:44:31 -0000
 @@ -54,52 +54,58 @@ int
  uname(name)
  	struct utsname *name;
  {
 -	int mib[2], rval;
 +	int mib[2];
  	size_t len;
  	char *p;
  
 -	rval = 0;
 -
  	_DIAGASSERT(name != NULL);
  
  	mib[0] = CTL_KERN;
  	mib[1] = KERN_OSTYPE;
  	len = sizeof(name->sysname);
  	if (sysctl(mib, 2, &name->sysname, &len, NULL, 0) == -1)
 -		rval = -1;
 +		goto error;
  
  	mib[0] = CTL_KERN;
  	mib[1] = KERN_HOSTNAME;
  	len = sizeof(name->nodename);
  	if (sysctl(mib, 2, &name->nodename, &len, NULL, 0) == -1)
 -		rval = -1;
 +		goto error;
  
  	mib[0] = CTL_KERN;
  	mib[1] = KERN_OSRELEASE;
  	len = sizeof(name->release);
  	if (sysctl(mib, 2, &name->release, &len, NULL, 0) == -1)
 -		rval = -1;
 +		goto error;
  
 -	/* The version may have newlines in it, turn them into spaces. */
  	mib[0] = CTL_KERN;
  	mib[1] = KERN_VERSION;
  	len = sizeof(name->version);
  	if (sysctl(mib, 2, &name->version, &len, NULL, 0) == -1)
 -		rval = -1;
 -	else
 -		for (p = name->version; len--; ++p) {
 -			if (*p == '\n' || *p == '\t') {
 -				if (len > 1)
 -					*p = ' ';
 -				else
 -					*p = '\0';
 -			}
 +		goto error;
 +
 +	/* The version may have newlines in it, turn them into spaces. */
 +	for (p = name->version; len--; ++p) {
 +		if (*p == '\n' || *p == '\t') {
 +			if (len > 1)
 +				*p = ' ';
 +			else
 +				*p = '\0';
  		}
 +	}
  
  	mib[0] = CTL_HW;
  	mib[1] = HW_MACHINE;
  	len = sizeof(name->machine);
  	if (sysctl(mib, 2, &name->machine, &len, NULL, 0) == -1)
 -		rval = -1;
 -	return (rval);
 +		goto error;
 +	return (0);
 +
 +error:
 +	name->sysname[0] = 0;
 +	name->nodename[0] = 0;
 +	name->release[0] = 0;
 +	name->version[0] = 0;
 +	name->machine[0] = 0;
 +	return (-1);
  }