xunzhang / gflags

Automatically exported from code.google.com/p/gflags
BSD 3-Clause "New" or "Revised" License
1 stars 0 forks source link

64 bit integer conversion type #13

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Configure your C++ compiler to treat all warnings as errors
2. Build on a 64 bit machine
3. Compile gflags

What is the expected output? What do you see instead?
Compiled code.
cc1plus: warnings being treated as errors
gflags.cc: In member function 'std::string google::FlagValue::ToString()
const':
gflags.cc:251: warning: format '%lld' expects type 'long long int', but
argument 4 has type 'long int'
gflags.cc:251: warning: format '%lld' expects type 'long long int', but
argument 4 has type 'long int'
gflags.cc:254: warning: format '%llu' expects type 'long long unsigned
int', but argument 4 has type 'long unsigned int'
gflags.cc:254: warning: format '%llu' expects type 'long long unsigned
int', but argument 4 has type 'long unsigned int'

What version of the product are you using? On what operating system?
gflags-0.8. Debian Etch.

Please provide any additional information below.
This error does not occur on 32 bit strict build systems, but it does on 64
bit.
Lines 251 and 254 in my version of gflags are as follows:

snprintf(intbuf, sizeof(intbuf), "%lld", VALUE_AS(int64));
snprintf(intbuf, sizeof(intbuf), "%llu", VALUE_AS(uint64));

Original issue reported on code.google.com by jtolds on 2 Jul 2008 at 8:03

GoogleCodeExporter commented 9 years ago
Thanks for the report.  I thought I had compiled this on a 64-bit machine before
releasing, but maybe not.

When I do try building on an x86_64 machine with gcc 4.2.2, I don't get the 
warnings
you mentioned.  We define int64 to be int64_t, which it seems to me should work 
with
%lld, but maybe it doesn't on some systems.

I know how to solve this kind of problem in general, but I wish I could 
reproduce the
problem to make sure my solution works.

Can you give more details on what OS and compiler you're using?

Original comment by csilv...@gmail.com on 10 Jul 2008 at 11:58

GoogleCodeExporter commented 9 years ago
This is a similar situation to issue 12. If you are using a build system that 
treats
warnings as errors on a 64 bit system (you can simulate this with the supplied 
build
system by running ./configure CXXFLAGS="-Wall -Werror -O2 -g" before make), 
besides
the signedness warnings in issue 12 you will receive the warning mentioned 
originally
in this issue.

The supplied build system (./configure && make) works just fine, but this issue 
is
for cases in which the supplied build system is replaced with another for 
various
reasons.

If this issue is a situation in which a given warning is something Google 
explicitly
ignores, that's okay, but perhaps this is something that can be cleanly fixed 
for
everyone.

Thanks. :)

Original comment by jtolds on 16 Jul 2008 at 12:13

GoogleCodeExporter commented 9 years ago
Hmm, not sure if it's the same as issue 12, since correctness could be affected 
in
this case.  But I'm not sure.

I can't reproduce the problem on my 64-bit system, so I'm not exactly sure what 
the
issue is, so I'm marking this Won't Fix for now.  Feel free to reopen if you 
start to
see problems with this.

Original comment by csilv...@gmail.com on 23 Jul 2008 at 12:24

GoogleCodeExporter commented 9 years ago
I'd re-open this if I knew how, but I know the source of this warning.
Using http://gcc.gnu.org/ml/gcc-help/2008-07/msg00029.html for my hunch, I 
believe
the problem is that on 64-bit systems, int_64_t is typedef'd to long int, not 
long
long int.

I notice on my system (debian/etch, gcc 4.3.1 ):
 sizeof(int) == 4
 sizeof(long int) == 8
 sizeof(long long int) == 8

So, luckily, the warning won't cause any problems.  But it appears that on 
64-bit
systems, the appropriate printf format would be "%ld", not "%lld".

Original comment by andrew.c...@gmail.com on 5 Aug 2008 at 3:10

GoogleCodeExporter commented 9 years ago
Indeed from /usr/include/stdint.h from the libc6-dev package on debian/testing 
amd64:

# if __WORDSIZE == 64
typedef long int        int64_t;
# else
__extension__
typedef long long int       int64_t;
#endif

Original comment by andrew.c...@gmail.com on 5 Aug 2008 at 3:15

GoogleCodeExporter commented 9 years ago
Gotcha.  OK, this is pretty easy to fix.  In the next gflags release, I'll 
replace
%lld with the PRId64 macro, which is standard and written to address just this 
problem.

Original comment by csilv...@gmail.com on 5 Aug 2008 at 10:19

GoogleCodeExporter commented 9 years ago
Fixed in gflags 1.0rc1.

Original comment by csilv...@gmail.com on 21 Aug 2008 at 12:56