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

i#1815: fix unit_test failure on macos sierra due to gettimeofday #2247

Merged
merged 3 commits into from
Mar 1, 2017

Conversation

shawndenbow
Copy link
Contributor

On MacOS before Sierra, gettimeofday returns usecs:secs and does not
set the timeval struct. Sierra correctly fills the timeval struct.

On MacOS before Sierra, gettimeofday returns usecs:secs and does not
set the timeval struct. Sierra correctly fills the timeval struct.
Copy link
Contributor

@derekbruening derekbruening left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing this. The code itself looks good, there's just an indentation issue and some simplifying suggestions.

# define MACOS_VERSION_YOSEMITE 14
# define MACOS_VERSION_MAVERICKS 13
# define MACOS_VERSION_MOUNTAIN_LION 12
# define MACOS_VERSION_LION 11
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Looks good static in this .c for now, but in the future we might want to consider exposing these via the dr_get_os_version() API.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree. I added in a comment to xref i#1404.

core/unix/os.c Outdated
/* MacOS before Sierra returns usecs:secs and does not set the timeval struct. */
if (macos_version < MACOS_VERSION_SIERRA) {
if ((int)val < 0)
return 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: indentation off

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

core/unix/os.c Outdated
if ((int)val > 0) {
current_time.tv_sec = (uint) val;
current_time.tv_usec = (uint)(val >> 32);
} else {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This else could be removed as we'll hit the else down below?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

core/unix/os.c Outdated
if ((int)val > 0) {
current_time.tv_sec = (uint) val;
current_time.tv_usec = (uint)(val >> 32);
} else {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This else could also be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@derekbruening derekbruening merged commit 38920e6 into master Mar 1, 2017
@derekbruening derekbruening deleted the i1815-macos-tests branch March 1, 2017 07:13
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.

2 participants