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

Not entirely portable use of usleep() in tests #15711

Closed
rsbeckerca opened this issue Dec 10, 2024 · 6 comments
Closed

Not entirely portable use of usleep() in tests #15711

rsbeckerca opened this issue Dec 10, 2024 · 6 comments

Comments

@rsbeckerca
Copy link
Contributor

I did this

Compiled curl with OpenSSL and c99 on NonStop x86, and got a compile error on usleep(100*1000). This function is only available in threaded code on NonStop. I need to use sleep(100) instead if not threaded.

Any objection if I put a pull request together for this?

I expected the following

Clean compile of the test suite.

curl/libcurl version

curl 8.11.0

operating system

NonStop L24.08

@vszakats
Copy link
Member

Any reason not to use sleep() on all platforms? (Except on Windows, which needs it remapped to Sleep())

@rsbeckerca
Copy link
Contributor Author

Any reason not to use sleep() on all platforms? (Except on Windows, which needs it remapped to Sleep())

I would have though that the technique used for Sleep() throughout the code would be better, but I'm somewhat confused as to how that even works. However, sleep() on NonStop does not appear to be thread blocking in all threading models, while usleep() has a safe wrapper. This is only in ws-data.c and ws-pingpong.c.

@rsbeckerca
Copy link
Contributor Author

My proposed PR is now up.

@bagder
Copy link
Member

bagder commented Dec 10, 2024

Any reason not to use sleep() on all platforms? (Except on Windows, which needs it remapped to Sleep())

Because POSIX sleep() is in number of seconds, usleep is microseconds. Apparently NonStop has a special sleep().

@vszakats
Copy link
Member

Any reason not to use sleep() on all platforms? (Except on Windows, which needs it remapped to Sleep())

Because POSIX sleep() is in number of seconds, usleep is microseconds. Apparently NonStop has a special sleep().

Ah, micro vs. milli got me!

@rsbeckerca
Copy link
Contributor Author

Any reason not to use sleep() on all platforms? (Except on Windows, which needs it remapped to Sleep())

Because POSIX sleep() is in number of seconds, usleep is microseconds. Apparently NonStop has a special sleep().

Ah, micro vs. milli got me!

Actually I meant milli, but sleep() is seconds.

rsbeckerca added a commit to rsbeckerca/curl that referenced this issue Dec 11, 2024
This fix impacts ws-data.c and ws-pingpong.c. sleep() replaced with system
PROCESS_DELAY_() having the same resolution as usleep().

Fixes: curl#15711

Signed-off-by: Randall S. Becker <randall.becker@nexbridge.ca>
@bagder bagder closed this as completed in aed732a Dec 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

3 participants