bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: rewritten inttypes module


From: Paul Eggert
Subject: Re: rewritten inttypes module
Date: Fri, 28 Jul 2006 04:26:37 -0400
User-agent: Gnus/5.1008 (Gnus v5.10.8) Emacs/21.4 (gnu/linux)

Bruno Haible <address@hidden> writes:

> Here, as well as in many other places (including 'stdint'), we assume that
> int and int32_t are the same. I don't wish to add more #ifdefs for cases
> that are hypothetical.

I was worried about the case where an implementation provides a
conforming stdint.h but not a conforming inttypes.h.  Such an
implementation might have 64-bit int.  There are a few ILP64 hosts,
e.g., GCC on MIPS with the -mint64 option.  Currently, I suspect that
all these platforms fail to have conforming stdint.h or inttypes.h
(since we are so picky about conformance) but it's possible we'll run
into one as conformance improves (i.e., we're not talking about old
UNIVACs here :-).

Instead of adding more ifdefs you could add a simple compile-time test
that INT_MIN == INT32_MIN && INT32_MAX == INT_MAX, and we can fill in
the blanks later, if/when we run into a host with 64-bit int.

>> I don't see why the changes to lib/stdint_.h, m4/stdint.m4, and
>> modules/stdint are needed.  They have an effect only if we use
>> gnulib's inttypes.h and gnulib's stdint.h.  If the program includes
>> inttypes.h first, this includes stdint.h, which in turn recursively
>> includes inttypes.h, but this innermost include is a noop because
>> INTTYPES_H is defined.
>
> Yes, and it shouldn't be a noop. It should include the system's inttypes.h
> file. Otherwise, if a user source code does
>
>      #include <inttypes.h>
>      #include </usr/include/inttypes.h>
>
> it leads to big havoc.

I don't see how the havoc could occur, since the substitute
<inttypes.h> includes /usr/include/inttypes.h before it includes
the substitute <stdint.h>.

> The abort() is precisely to protect against such old implementations.
> If we encounter an implementation where % on negative numbers works
> not like C99 says, we have to deal not only in imaxdiv but also / and %.
> I hope this will never be necessary, that's why I put the abort() there.

OK, I see; perhaps that should be commented then.  Maybe a comment like this?

  /* This code checks that / and % work on integers the way that C99 says
     (i.e., Fortran-like integer division).  C89 does not specify
     whether ceiling or floor is taken for division when either operand is
     negative.  We know of no practical C89 host that violates the C99
     rules; if this host is a counterexample, bad things can happen
     here and probably elsewhere in the program.  Abort so that we are
     more likely to be informed of the porting-related bug.  */

One other nit:

      if (!(numer >= 0
            ? result.rem >= 0 && result.rem < (denom >= 0 ? denom : - denom)
            : result.rem <= 0 && result.rem > (denom >= 0 ? - denom : denom)))

This has undefined behavior if denom == INTMAX_MIN.  How about
something like this instead?

      if (!(numer >= 0
            ? (result.rem >= 0
               && (denom >= 0 ? result.rem < denom : - result.rem > denom))
            : (result.rem <= 0
               && (denom >= 0 ? - result.rem < denom : result.rem > denom))))




reply via email to

[Prev in Thread] Current Thread [Next in Thread]