Re: large integer truncation in regex module

[Jim Meyering]

Gianluigi Tiesi wrote:

> On 26/03/2012 7.33, Jim Meyering wrote:
>> Gianluigi Tiesi wrote:
>>> On 25/03/2012 6.58, Paul Eggert wrote:
>>>> On 03/24/2012 09:28 PM, Gianluigi Tiesi wrote:
>>>>> While compiling regex module one android I've discovered a problem
>>>>> that on other 32bit compiler is only a warning
>>>>>
>>>>> ...
>>>>> it's correct, but I think an ifdef may be used instead since
>>>>> BITSET_WORD_BITS is a define
>>>>
>>>>
>>>> Generally speaking we prefer 'if (xxx)' to '#if xxx' where either will
>>>> do, because the former is easier to read and reason about. If the only
>>>> problem with 'if (xxx)' is a bogus warning by some random compiler then
>>>> it's probably better to leave it alone (and get the compiler fixed....).
>>>
>>> I don't think assigning a 64bit integer constant in a 32bit variable
>>> type is a bogus warning,
>>
>> It is bogus in this case because your compiler is warning
>> about what is called dead code: code that it can determine
>> will never be executed.
>
> hmm, if gcc is unable to detect dead code, this does not make legit
> everything in the dead code.
>
>>
>>> and some software using gnulib like parted
>>> enable Werror by default

No package should be forcing you to use -Werror.

>> Gnu parted does not enable -Werror by default.
>> However, it is enabled if you configure with --enable-gcc-warnings.
>
> gnu parted 2.4 enables by default -Werror, you can configure it with
> --enable-Werror=no, git version (3.1) removed at all the option and the

I know.  I removed that code 8 months ago, before parted-3.0.

> flag. But I still don't understand why use dead code instead of ifdef
> if the compiler is unable to detect it.

Paul already explained above: it is more readable.
It doesn't warn for us because we use more recent compilers.

> I've tested with gcc 4.4.3
> (android ndk) and 4.6.2 (mingw), both warns.
> I'm not really sure even if it warns that the code is compiled in the 
> executable.
>
> I've just tested the code:
>
> #include <stdio.h>
> #include <stdlib.h>
> #define A 64
> //#define A 32
>
> int main(void)
> {
>     int a = 0;
>     if (A == 32)
>         a = ((unsigned long long) (-1));
>     if (a)
>         printf("%d\n", a);
>     return 0;
> }
>
> it warns about an overflow, but if variable a is not initialized, it says it 
> is used uninitialized (not maybe used)
>
> a.c: In function 'main':
> a.c:10:9: warning: overflow in implicit constant conversion [-Woverflow]
> a.c:11:8: warning: 'a' is used uninitialized in this function 
> [-Wuninitialized]
>
> if you compile with -O2 and look at the asm the printf is removed,
> but you still have a warn.
>
> Ok I went a bit OT :) anyway it's uncommon for me to see if (DEFINE) else etc

We take attention to style/maintainability/readability further than most.