Subject: Re: lib/35403: Error code path optimization in libc's implementation of uname()
To: None <gnats-bugs@NetBSD.org, netbsd-bugs@netbsd.org>
From: Christian Biere <christianbiere@gmx.de>
List: netbsd-bugs
Date: 01/15/2007 22:00:00
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);
 }