From owner-cvs-src@FreeBSD.ORG Sat Sep 22 20:24:08 2007 Return-Path: Delivered-To: cvs-src@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 0BDCA16A417; Sat, 22 Sep 2007 20:24:08 +0000 (UTC) (envelope-from scf@FreeBSD.org) Received: from mail.farley.org (farley.org [67.64.95.201]) by mx1.freebsd.org (Postfix) with ESMTP id CEA4D13C448; Sat, 22 Sep 2007 20:24:07 +0000 (UTC) (envelope-from scf@FreeBSD.org) Received: from thor.farley.org (thor.farley.org [192.168.1.5]) by mail.farley.org (8.14.1/8.14.1) with ESMTP id l8MJwqFw034844; Sat, 22 Sep 2007 14:58:52 -0500 (CDT) (envelope-from scf@FreeBSD.org) Date: Sat, 22 Sep 2007 14:58:52 -0500 (CDT) From: "Sean C. Farley" To: Bruce Evans In-Reply-To: <20070922202914.B90809@besplex.bde.org> Message-ID: References: <200709220230.l8M2UiRK020609@repoman.freebsd.org> <86r6krqbrd.fsf@ds4.des.no> <20070922202914.B90809@besplex.bde.org> MIME-Version: 1.0 Content-Type: MULTIPART/MIXED; BOUNDARY="0-1449508764-1190491117=:52204" Content-ID: X-Spam-Status: No, score=-4.4 required=5.0 tests=ALL_TRUSTED,AWL,BAYES_00 autolearn=ham version=3.2.3 X-Spam-Checker-Version: SpamAssassin 3.2.3 (2007-08-08) on mail.farley.org Cc: =?ISO-8859-15?Q?Dag-Erling_Sm=F8rgrav?= , src-committers@FreeBSD.org, cvs-all@FreeBSD.org, cvs-src@FreeBSD.org Subject: Re: cvs commit: src/lib/libc/stdlib getenv.c X-BeenThere: cvs-src@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: CVS commit messages for the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 22 Sep 2007 20:24:08 -0000 This message is in MIME format. The first part should be readable text, while the remaining parts are likely unreadable without MIME-aware tools. --0-1449508764-1190491117=:52204 Content-Type: TEXT/PLAIN; CHARSET=UTF-8; FORMAT=flowed Content-Transfer-Encoding: 8BIT Content-ID: On Sat, 22 Sep 2007, Bruce Evans wrote: > On Sat, 22 Sep 2007, [utf-8] Dag-Erling Smørgrav wrote: > >> Sean Farley writes: >>> Log: >>> The precision for a string argument in a call to warnx() needs to >>> be cast to an int to remove the warning from using a size_t >>> variable on 64-bit platforms. >> >> s/to remove the warning/to actually work/ I do agree with this; I consider warnings to be nests for bugs to hide. I also dislike casts in general since they can hide bugs too. This is why I wish the specification for the printf() family of functions, which warnx uses, required the precision to be size_t instead of int to match the output type of strlen(). Unfortunately, it would be ssize_t since it accepts negative values. > Please be precise :-). > > s/to remove the warning ... on 64-bit platforms/to avoid undefined > behaviour on platforms where size_t is not u_int, and to avoid having > to make a delicate analysis to show that the behaviour is defined and > correct on all other platforms/. Thank you for the analysis below. You are a wealth of standard/specification knowledge. > Delicate analysis: > - size_t is always an unsigned type, but the required type is int, so > size_t is never compatible with the required type. > - on platforms where size_t is smaller than int, the arg type is > nevertheless compatible with int, since warnx() is variadic and the > arg is one of the variadic args; the default promotions thus apply > and the arg is passed as an int whether or not you cast it > explicitly to int (but casting it to a type larger than int would > break it). FreeBSD doesn't support any platforms in this class. > - on platforms where size_t is u_int, the arg is passed as a u_int. > The analysis for this case is too delicate to give in full here. > Partial analysis: > - the size_t variable must have a small value that is representable > as an int (else casting it to int would be a bug and/or printing a > line of that length would be a style bug). What would be a good maximum that would fit style? Although still fairly big, NL_TEXTMAX for the entire line looks plausible. > - the behaviour seems to have been undefined in C90, since va_arg() > requires strict type compatibility in C90 and warnx() is > implemented using va_arg(ap, int) which gave UB on u_int's. > Similarly for function calls, except the wording is less > clear/strict. > - UB in C90 was a bug in C90. This is fixed in C99. Now both > va_arg() and function call args are specifically required to work > if one type is a signed integer type, the [promotion of the] other > type is the corresponding unsigned integer type, and the value is > representable in both types. Compatibility of the representation > of integers and unsigned integers probably also requires this, but > the specification of this in C90 is probably to fuzzy to override > the parts that specify UB. Everyone just knows that this case has > to work. I must be slow today; it took me awhile to see UB as undefined behavior. Would the best solution be to place a cap on the value? If the value is less than INT_MAX, cast it to an int else pass it INT_MAX. Actually, it looks like the value should never be greater than ARG_MAX if wanting to be able to call exec since according to SUSv3 that is the: Maximum length of argument to the exec functions including environment data. Hopefully, no environment variables (name=value string) are anywhere close in size to size_t. :) Here is a patch (untested) to at least cast safely. How does this look? Index: getenv.c =================================================================== RCS file: /home/ncvs/src/lib/libc/stdlib/getenv.c,v retrieving revision 1.12 diff -u -r1.12 getenv.c --- getenv.c 22 Sep 2007 02:30:44 -0000 1.12 +++ getenv.c 22 Sep 2007 19:05:51 -0000 @@ -356,8 +356,8 @@ activeNdx = envVarsTotal - 1; if (__findenv(envVars[envNdx].name, nameLen, &activeNdx, false) == NULL) { - warnx(CorruptEnvFindMsg, (int)nameLen, - envVars[envNdx].name); + warnx(CorruptEnvFindMsg, nameLen > INT_MAX ? INT_MAX : + (int)nameLen, envVars[envNdx].name); errno = EFAULT; goto Failure; } Sean -- scf@FreeBSD.org --0-1449508764-1190491117=:52204--