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

Remove compiler warning in logging.cc #459

Closed
wants to merge 1 commit into from

Conversation

jonricha
Copy link

There is an unsigned vs signed comparison warning in logging.cc. This should remove it. It is already guaranteed to be a positive number from the condition on line 53 where c is already known to be between '0' and '9', so c will be in the range of 0 to 9.

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@jonricha
Copy link
Author

I signed it

@googlebot
Copy link

CLAs look good, thanks!

@maflcko
Copy link
Contributor

maflcko commented Mar 19, 2018

Can be closed, since it is included in #492 and needs rebase.

@pwnall
Copy link
Member

pwnall commented Apr 14, 2018

@jonricha Thank you for your contribution!

I didn't merge it because the old implementation of ConsumeDecimalNumber crashed inexplicably on ARM until we changed the type of a local variable, so I was afraid that a similar type change might bring the crashes back. I ended up reimplementing ConsumeDecimalNumber and this PR doesn't make sense in the context of the new implementation.

Also, sorry for the late response.

@pwnall pwnall closed this Apr 14, 2018
maochongxin pushed a commit to maochongxin/leveldb that referenced this pull request Jul 21, 2022
…oogle#459)

Recently the library added a new ranged-for variant of the KeepRunning
loop that is much faster. For this reason it should be preferred in all
new code.

Because a library, its documentation, and its tests should all embody
the best practices of using the library, this patch changes all but a
few usages of KeepRunning() into for (auto _ : state).

The remaining usages in the tests and documentation persist only
to document and test behavior that is different between the two formulations.

Also note that because the range-for loop requires C++11, the KeepRunning
variant has not been deprecated at this time.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants