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

Replace ftime() by clock_gettime() #276

Closed
wants to merge 8 commits into from
Closed

Replace ftime() by clock_gettime() #276

wants to merge 8 commits into from

Conversation

mreininghaus
Copy link
Contributor

ftime() is considered obsolete in the POSIX standard and not available anymore on some platforms, e.g. OpenBSD. Therefore, I replaced it by clock_gettime(), however keeping ftime() as fallback.

@coveralls
Copy link

coveralls commented Mar 30, 2018

Coverage Status

Coverage decreased (-0.02%) to 49.492% when pulling 42762cb on mreininghaus:ftime-clock_gettime into cc3cbd0 on vermaseren:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 49.475% when pulling b47169d on mreininghaus:ftime-clock_gettime into e900894 on vermaseren:master.

@tueda
Copy link
Collaborator

tueda commented Mar 31, 2018

Thank you for your pull request! I totally agree with you that ftime() is obsolete and it should be replaced at some point. Some comments:

  • Probably checking clock_gettime() in the configure is needed. Some system may require linking to rt and this is the reason why the CI failed. Once the check is done via configure, _POSIX_TIMERS is replaced with HAVE_CLOCK_GETTIME or something like that. For reference, my attempt for gettimeofday() is here.
  • Because this is a set of rather small patches, would you mind squashing them into one commit and rebasing to the master such that fast-forward merge would work?

@mreininghaus
Copy link
Contributor Author

Thank you for your comments. I'll implement your suggestions.

@tueda
Copy link
Collaborator

tueda commented Aug 6, 2018

@mreininghaus Sorry that I didn't look into this for quite a while.

Do you still want to edit your commit history? Especially your 3 merge commits nonlinearly complicates the commit graph.

If you don't mind, I could git-cherry-pick your commits without the merge commits. The result will be like https://github.com/tueda/form/commits/mreininghaus-ftime-clock_gettime and this can be merged into the master branch.

@mreininghaus
Copy link
Contributor Author

Feel free to do that. Sorry if I messed things up a bit.

@tueda
Copy link
Collaborator

tueda commented Aug 11, 2018

OK, I will try to merge it in that way. Thanks for your contribution!

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.

3 participants