From owner-freebsd-bugs@FreeBSD.ORG Mon Jun 28 19:50:03 2010 Return-Path: Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 8B5521065670 for ; Mon, 28 Jun 2010 19:50:03 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 6071F8FC08 for ; Mon, 28 Jun 2010 19:50:03 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.4/8.14.4) with ESMTP id o5SJo3EP038136 for ; Mon, 28 Jun 2010 19:50:03 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.4/8.14.4/Submit) id o5SJo3qW038135; Mon, 28 Jun 2010 19:50:03 GMT (envelope-from gnats) Date: Mon, 28 Jun 2010 19:50:03 GMT Message-Id: <201006281950.o5SJo3qW038135@freefall.freebsd.org> To: freebsd-bugs@FreeBSD.org From: Garrett Cooper Cc: Subject: Re: kern/144307: ENOENT set unnecessarily under certain circumstances when malloc is called / fails X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Garrett Cooper List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 28 Jun 2010 19:50:03 -0000 The following reply was made to PR kern/144307; it has been noted by GNATS. From: Garrett Cooper To: Jaakko Heinonen Cc: bug-followup@freebsd.org Subject: Re: kern/144307: ENOENT set unnecessarily under certain circumstances when malloc is called / fails Date: Mon, 28 Jun 2010 12:46:17 -0700 On Mon, Jun 28, 2010 at 12:05 PM, Jaakko Heinonen wrote: > On 2010-02-26, Garrett Cooper wrote: >> On systems where /etc/malloc.conf isn't present, some failures >> syscalls like read will fail incorrectly with ENOENT because the file >> doesn't exist, and thus output via errx will be incorrect, like shown > > I think you mean err(3), not errx(3). Yeah... true :). >> from the following disklabel output: >> >> 1+0 records in >> 1+0 records out >> 512 bytes transferred in 0.000054 secs (9502140 bytes/sec) >> disklabel: /dev/md1 read: No such file or directory > > It's a disklabel bug to inspect the errno value in the success case. > POSIX states: "The value of errno should only be examined when it is > indicated to be valid by a function's return value." and "The setting of > errno after a successful call to a function is unspecified unless the > description of that function specifies that errno shall not be > modified.". > > This patch for bsdlabel(8) might fix the error message: > > %%% > Index: sbin/bsdlabel/bsdlabel.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- sbin/bsdlabel/bsdlabel.c =A0 =A0(revision 209580) > +++ sbin/bsdlabel/bsdlabel.c =A0 =A0(working copy) > @@ -312,7 +312,7 @@ static void > =A0fixlabel(struct disklabel *lp) > =A0{ > =A0 =A0 =A0 =A0struct partition *dp; > - =A0 =A0 =A0 int i; > + =A0 =A0 =A0 ssize_t i; Good catch on the implicit cast. > =A0 =A0 =A0 =A0for (i =3D 0; i < lp->d_npartitions; i++) { > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (i =3D=3D RAW_PART) > @@ -359,8 +359,11 @@ readboot(void) > =A0 =A0 =A0 =A0fstat(fd, &st); > =A0 =A0 =A0 =A0if (alphacksum && st.st_size <=3D BBSIZE - 512) { > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0i =3D read(fd, bootarea + 512, st.st_size)= ; > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (i !=3D st.st_size) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (i =3D=3D -1) > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0err(1, "read error %s", xx= boot); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (i !=3D st.st_size) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 errx(1, "couldn't read %ju = bytes from %s", > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (uintmax_t)st.st_si= ze, xxboot); > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 * Set the location and length so SRM can = find the > @@ -373,8 +376,11 @@ readboot(void) > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return; > =A0 =A0 =A0 =A0} else if ((!alphacksum) && st.st_size <=3D BBSIZE) { > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0i =3D read(fd, bootarea, st.st_size); > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (i !=3D st.st_size) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (i =3D=3D -1) > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0err(1, "read error %s", xx= boot); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (i !=3D st.st_size) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 errx(1, "couldn't read %ju = bytes from %s", > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (uintmax_t)st.st_si= ze, xxboot); Or the malloc(3) call could be fixed with the couple of lines I noted (well, adlibbed of course... I wrote the patch before I started familiarizing myself with style(9))? > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return; > =A0 =A0 =A0 =A0} > =A0 =A0 =A0 =A0errx(1, "boot code %s is wrong size", xxboot); > @@ -499,7 +505,7 @@ readlabel(int flag) > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0"disks with more than 2^32-1 secto= rs are not supported"); > =A0 =A0 =A0 =A0(void)lseek(f, (off_t)0, SEEK_SET); > =A0 =A0 =A0 =A0if (read(f, bootarea, BBSIZE) !=3D BBSIZE) > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 err(4, "%s read", specname); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 errx(4, "couldn't read %d bytes from %s", B= BSIZE, specname); > =A0 =A0 =A0 =A0close (f); > =A0 =A0 =A0 =A0error =3D bsd_disklabel_le_dec( > =A0 =A0 =A0 =A0 =A0 =A0bootarea + (labeloffset + labelsoffset * secsize), > %%% Which I agree with, but shouldn't we fix malloc(3) (and any other function calls that depend on malloc(3) for sensible results)? Thanks! -Garrett PS Neither malloc nor read mentions ENOENT in their respective manpages (and if they did it would be non-POSIX compliant - http://www.opengroup.org/onlinepubs/009695399/functions/malloc.html , http://opengroup.org/onlinepubs/007908775/xsh/read.html).