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

timeval: introduce curlval as a struct timeval replacement #1693

Closed
wants to merge 6 commits into from

Conversation

bagder
Copy link
Member

@bagder bagder commented Jul 21, 2017

... to make all libcurl internals able to use the same data types for
the struct members. The timeval struct differs subtly on several
platforms so it makes it cumbersome to use everywhere.

Ref: #1652

... to make all libcurl internals able to use the same data types for
the struct members. The timeval struct differs subtly on several
platforms so it makes it cumbersome to use everywhere.

Ref: #1652
@coveralls
Copy link

Coverage Status

Coverage decreased (-75.3%) to 0.0% when pulling 4c5b3ff on bagder/timeval-curlval into 42a4cd4 on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-49.2%) to 26.069% when pulling 0540b34 on bagder/timeval-curlval into 42a4cd4 on master.

@@ -48,7 +48,7 @@ struct timeval curlx_tvnow(void)

Copy link
Member

Choose a reason for hiding this comment

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

I agree this is the right move. What do you think about a name like curltime instead of curlval to know it's time related? The (long)s above need to be changed to time_t and unsigned, otherwise looks fine

Copy link
Member

Choose a reason for hiding this comment

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

Changes I needed to get it working in Visual Studio are at bagder/timeval-curlval...jay:pr_1693_amended

jay added a commit to jay/curl that referenced this pull request Jul 25, 2017
- Revert some curlval changes in select.c since select uses timeval.

- Various typecasts elsewhere.

Ref: curl#1693
@bagder
Copy link
Member Author

bagder commented Jul 26, 2017

Renamed the struct to 'curltime', merged @jay's changes (manually).

I plan to merge these commits squashed once the CI agrees they're fine.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 75.308% when pulling a9a8fa4 on bagder/timeval-curlval into 42a4cd4 on master.

@bagder bagder closed this in 4dee50b Jul 28, 2017
@bagder bagder deleted the bagder/timeval-curlval branch July 28, 2017 14:37
jay added a commit that referenced this pull request Jul 29, 2017
@lock lock bot locked as resolved and limited conversation to collaborators Jan 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants