On Sat, 22 Sep 2007, Bruce Evans wrote:

On Sat, 22 Sep 2007, [utf-8] Dag-Erling Smørgrav wrote:

Sean Farley <[EMAIL PROTECTED]> 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
--
[EMAIL PROTECTED]
_______________________________________________
cvs-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/cvs-all
To unsubscribe, send any mail to "[EMAIL PROTECTED]"

Reply via email to