I hate to quibble on this, but strictly speaking:

 for (int i = 0; i < some_vector.size(); i++)

is not perfectly valid unless you have verified that some_vector.size() <
static_cast<size_t>(std::numeric_limits<int>::max());

This would be broken in cases of a very large vector (possible with a
vector<unsigned char> on 32-bit Linux or that abomination vector<bool>
almost anywhere), or if you had a platform with sizeof(int) <
sizeof(size_t), which is surprisingly common (for example, on Solaris:
http://docsun.cites.uiuc.edu/sun_docs/C/solaris_9/SUNWdev/SOL64TRANS/p6.html#CHAPTER3-TBL-3
).

Sometimes, compilers warn about really dangerous things.

--Chris

On Mon, Jan 11, 2010 at 10:35 AM, Kenton Varda <ken...@google.com> wrote:

> 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<protobuf%2bunsubscr...@googlegroups.com>
> .
> For more options, visit this group at
> http://groups.google.com/group/protobuf?hl=en.
>
>


-- 
Chris
--
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