Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MurmurIce.cpp: Fix comparison between signed and unsigned integer #2938

Conversation

@davidebeatrici
Copy link
Member

commented Mar 13, 2017

MurmurIce.cpp:48: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
  for (int i = 0; i < s.size(); i++) {
                  ~~^~~~~~~~~~
@@ -45,7 +45,7 @@ void IceStop() {
/// Remove all NUL bytes from |s|.
static std::string iceRemoveNul(std::string s) {
std::vector<char> newstr;
for (int i = 0; i < s.size(); i++) {
for (unsigned int i = 0; i < s.size(); i++) {

This comment has been minimized.

Copy link
@mkrautz

mkrautz Mar 13, 2017

Member

Should be size_t instead of unsigned int.

This comment has been minimized.

Copy link
@davidebeatrici

davidebeatrici Mar 13, 2017

Author Member

Fixed.

@davidebeatrici davidebeatrici force-pushed the davidebeatrici:MurmurIce.cpp_unsigned-signed-int-comparison branch from f2a90c1 to c995924 Mar 13, 2017

@Kissaki

This comment has been minimized.

Copy link
Member

commented Mar 14, 2017

Please remove the warning from the commit message. That's something you should have noticed yourself after touching this again after the other reviews.

@Kissaki

This comment has been minimized.

Copy link
Member

commented Mar 14, 2017

Personally, I would make it:

Fix signed/unsigned comparison warning

No need for a description. That's clear enough.

It's also a lot shorter - I don't care about it being an integer, and the slash is clear enough here. Yours is way longer than the recommended (max) of 56 characters. Mine fits even with the file name (which I'm not entirely sure I see the point of, but np, we use them elsewhere, used them on several commits).

MurmurIce.cpp: Fix comparison between signed and unsigned integer

66 characters

MurmurIce.cpp: Fix signed/unsigned comparison warning

54 characters

@davidebeatrici davidebeatrici force-pushed the davidebeatrici:MurmurIce.cpp_unsigned-signed-int-comparison branch from c995924 to 02b1ddb Mar 14, 2017

@davidebeatrici davidebeatrici force-pushed the davidebeatrici:MurmurIce.cpp_unsigned-signed-int-comparison branch from 02b1ddb to f7798c3 Mar 14, 2017

@mkrautz mkrautz merged commit 649537f into mumble-voip:master Mar 14, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.