[runtime] Fix potential overflow when using mono_msec_ticks #2721
Conversation
|
For the case in |
|
Sorry, not in terms of |
luhenry
force-pushed
the
fix-mono_ms_ticks
branch
2 times, most recently
from
af9ab49
to
ad46d09
Compare
|
@alexrp I updated this PR to have |
luhenry
force-pushed
the
fix-mono_ms_ticks
branch
from
ad46d09
to
000248a
Compare
|
Looks OK to me. @kumpera: could you review also? |
|
|
||
| #ifdef HOST_WIN32 | ||
| #include <windows.h> | ||
| #include <winbase.h> |
There was a problem hiding this comment.
winbase.h isn't meant to be included directly, windows.h above should be enough.
luhenry
force-pushed
the
fix-mono_ms_ticks
branch
from
000248a
to
ff244a9
Compare
|
build |
| guint32 | ||
| mono_msec_ticks (void) | ||
| /* we get "error: implicit declaration of function 'GetTickCount64'" */ | ||
| ULONGLONG GetTickCount64(void); |
There was a problem hiding this comment.
You should wrap this into a #ifndef _MSC_VER to make sure we only get this on cygwin/mingw, otherwise we'll get errors on the VS build.
Because mono_msec_ticks would previously return the number of milliseconds since boot time, it would overflow after ~49 days. That would mean that anywhere you would use it, you would need to be careful of arithmetic overflow. By simply returning a gint64 we will not overflow before ~30k years. Fixes bug https://bugzilla.xamarin.com/show_bug.cgi?id=38666
luhenry
force-pushed
the
fix-mono_ms_ticks
branch
from
ff244a9
to
6277ba5
Compare
|
@kumpera please review :) |
|
LGTM. please fix the cygwin issue mentioned by @akoeplinger |
|
@lewurm the issue was fixed, GitHub just doesn't collapse the commit because the line didn't change. |
|
oh and please note that if you rebase the Windows build will be red anyway because we have a general issue there right now |
|
@ludovic-henry, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR. |
|
@kumpera could you please review? |
|
build |
[runtime] Fix potential overflow when using mono_msec_ticks Commit migrated from mono/mono@6b139f4
No description provided.