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);
}