We get a lot of complaints about warnings in our code.  When the warnings
occur in generated code, I generally fix them (because generated code
generally must be compiled using your project's flags), but for warnings in
protobuf code my answer is that you need to include the protobuf headers as
system headers so that warnings are ignored.  Different projects make
different choices about what warnings to enable on their code, so it seems
unreasonable to expect that every one of your dependencies enable at least
all of the warnings that you enable.  The sign comparison warnings are
particularly annoying because they warn about every line that looks like:
  for (int i = 0; i < some_vector.size(); i++)
which are very common and perfectly valid.  Therefore we compile with
-fno-sign-compare.

That said, this patch seems harmless, so I'll submit it.  But I won't be
doing a new release just for this, and new warnings could easily be
introduced before the next release.

On Sun, Jan 10, 2010 at 3:13 AM, edan <eda...@gmail.com> wrote:

> I looks like a good workaround - thanks for the info.
> I will wait and see if Kenton is planning to fix this, then decide my next
> steps.
> --edan
>
>
> On Sun, Jan 10, 2010 at 10:59 AM, Monty Taylor <mord...@inaugust.com>wrote:
>
>>
>>
>> edan wrote:
>> > I happily upgraded to 2.3.0 - I always like to take the latest and
>> greatest.
>> > Unfortunately, and I think for the first time ever while upgrading
>> > protobuf, I ran into a problem!
>> > We compile our code with "-Werror", and this bombed out on a header
>> file:
>>
>> We build with errors on in our project too - our solution to this has
>> been to:
>>
>> a) install the headers into a system locatiom, at which point gcc will
>> not issue warnings for them. It looks like you did this in the context
>> of your /devfs location - perhaps you need to change some system configs
>> to completely understand that location as a chroot?
>>
>> b) If they aren't in a system location, include them via -isystem rather
>> than -I, which will have the same effect.
>>
>> > cc1plus: warnings being treated as errors
>> > ../../../devfs/usr/include/google/protobuf/io/coded_stream.h: In member
>> > function "bool
>> >
>> google::protobuf::io::CodedInputStream::ReadLittleEndian32(google::protobuf::uint32*)":
>> > ../../../devfs/usr/include/google/protobuf/io/coded_stream.h:776:
>> > warning: comparison between signed and unsigned integer expressions
>> > ../../../devfs/usr/include/google/protobuf/io/coded_stream.h: In member
>> > function "bool
>> >
>> google::protobuf::io::CodedInputStream::ReadLittleEndian64(google::protobuf::uint64*)":
>> > ../../../devfs/usr/include/google/protobuf/io/coded_stream.h:791:
>> > warning: comparison between signed and unsigned integer expressions
>> >
>> >
>> > My patch to fix this was:
>> >
>> > ====
>> >
>> //depot/project/zenith/ports/protobuf/std/build/src/google/protobuf/io/coded_stream.h#2
>> > (ktext) -
>> >
>> //depot/project/zenith/ports/protobuf/std/build/src/google/protobuf/io/coded_stream.h#3
>> > (ktext) ==== content
>> > 776c776
>> > <   if (GOOGLE_PREDICT_TRUE(BufferSize() >= sizeof(*value))) {
>> > ---
>> >>   if (GOOGLE_PREDICT_TRUE(BufferSize() >=
>> > static_cast<int>(sizeof(*value)))) {
>> > 791c791
>> > <   if (GOOGLE_PREDICT_TRUE(BufferSize() >= sizeof(*value))) {
>> > ---
>> >>   if (GOOGLE_PREDICT_TRUE(BufferSize() >=
>> > static_cast<int>(sizeof(*value)))) {
>> >
>> > Any chance you can patch this and re-release?  I'd really like to have
>> > un-patched code in our product, but I can't use 2.3.0 without this
>> patch.
>>
>
>
--
You received this message because you are subscribed to the Google Groups "Protocol Buffers" group.
To post to this group, send email to proto...@googlegroups.com.
To unsubscribe from this group, send email to protobuf+unsubscr...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/protobuf?hl=en.

Reply via email to