xunzhang / gflags

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

Comparison between signed and unsigned types #12

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Configure C++ compiler to treat all warnings as errors
2. Compile gflags

What is the expected output? What do you see instead?
Compiled code. Error: comparison between signed and unsigned integer
expressions

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

Please provide any additional information below.
Hey Craig, all,
 Probably not a big issue, obviously easy to fix, but I'm using gflags in
an environment where there were a few changes that had to be made before it
would be compiled properly (detailed above). I've attached a patch, though
it only touches gflags.cc and gflags_reporting.cc, and no unit tests.
Furthermore, I fixed gflags_reporting.cc by converting just one unsigned
type to signed, when it really makes more sense to change all values that a
sign doesn't make sense on to unsigned, such as kLineLength.
:)

Original issue reported on code.google.com by jtolds on 2 Jul 2008 at 3:54

Attachments:

GoogleCodeExporter commented 9 years ago
When I compile gflags using the given Makefile, I don't get any warnings.  I 
take it
you do?  How exactly did you configure and compile gflags?  I only have access 
to an
ubuntu 6 machine, so it's a bit difficult for me to reproduce the problem.

Based on the patch, it looks like the problem you saw is one between signed and
unsigned types, which is a check we explicitly ignore at google (basically 
because we
don't trust the behavior of unsigned types, so use signed whenever possible, 
but libc
makes some things unsigned which leads to these warnings, which are always 
harmless).

In other projects, we turn it off with an explicit -Wno-signed-compare, which 
we add
when we notice we're compiling under gcc.  Maybe it's worth it to do that here? 
 We
could fix all these signedness warnings, but my bias is not to, since I think it
makes the code more fragile rather than less.

I'll fix this in the next release by explicitly adding -Wno-signed-compare to 
the
Makefile.  I hope that's an ok solution for you. :-)

Original comment by csilv...@gmail.com on 10 Jul 2008 at 8:53

GoogleCodeExporter commented 9 years ago
Oh, alright. I don't think there's any change necessary, then. I wasn't using 
the
given Makefile, as I had rolled the code into the codebase we are working with. 
I had
just pulled out the necessary source files and built my own makefiles using our
existing makefile system. Our makefile system treats all warnings as errors, as 
I
mentioned, and so the signedness warnings cause the build to fail.

I will consider instead requesting that we either ignore such errors or add the
option you suggest.

Thanks for your help. I don't think the makefile needs to be changed.

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

GoogleCodeExporter commented 9 years ago
I've added the Makefile flags to gflags 0.9, just released.

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