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

Large integers get truncated #594

Closed
r-barnes opened this issue Oct 15, 2020 · 3 comments · Fixed by #595
Closed

Large integers get truncated #594

r-barnes opened this issue Oct 15, 2020 · 3 comments · Fixed by #595
Assignees
Labels
Milestone

Comments

@r-barnes
Copy link
Contributor

r-barnes commented Oct 15, 2020

In our application we have some loggers that trigger every millisecond.

This is problematic since LogMessage uses a signed 32-bit integer as the counter type (see here) which can lead to a wrap every 24.85. This is bad for long-running processes.

It would probably be better if LogMessage used uint64_t or size_t as the counter type.

If there's a size concern, then using uint32 as the data type would at least double the time before wrap around.

@daohu527
Copy link

int will wrap after 24.85 days. Seems there is no logical problem, but counting number will wrong.

@ot
Copy link

ot commented Oct 19, 2020

@daohu527 Signed overflow is undefined behavior in C++, and the type here is an int.

@daohu527
Copy link

daohu527 commented Oct 20, 2020

LOG_EVERY_N

They use two parameters to implement this logic. LOG_OCCURRENCES_MOD_N will not great than N+1 to filter the log. And LOG_OCCURRENCES will use to count the log num.

glog/src/glog/logging.h.in

Lines 908 to 915 in c8f8135

#define SOME_KIND_OF_LOG_EVERY_N(severity, n, what_to_do) \
static int LOG_OCCURRENCES = 0, LOG_OCCURRENCES_MOD_N = 0; \
++LOG_OCCURRENCES; \
if (++LOG_OCCURRENCES_MOD_N > n) LOG_OCCURRENCES_MOD_N -= n; \
if (LOG_OCCURRENCES_MOD_N == 1) \
@ac_google_namespace@::LogMessage( \
__FILE__, __LINE__, @ac_google_namespace@::GLOG_ ## severity, LOG_OCCURRENCES, \
&what_to_do).stream()

So I said there is no problem with logic, LOG_OCCURRENCES int will overflow is true, but LOG_OCCURRENCES_MOD_N not.
There maybe a little problem too, n should be set in [1, INT_MAX-1]

@sergiud sergiud linked a pull request Mar 31, 2021 that will close this issue
@sergiud sergiud self-assigned this Mar 31, 2021
@sergiud sergiud added the bug label Mar 31, 2021
@sergiud sergiud added this to the 0.5 milestone Mar 31, 2021
@sergiud sergiud mentioned this issue May 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants