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

Fix usleep argument #444

Merged
merged 1 commit into from
Nov 30, 2016
Merged

Fix usleep argument #444

merged 1 commit into from
Nov 30, 2016

Conversation

0-wiz-0
Copy link
Contributor

@0-wiz-0 0-wiz-0 commented Nov 30, 2016

According to POSIX (http://pubs.opengroup.org/onlinepubs/009695399/functions/usleep.html):
The useconds argument shall be less than one million.
Make it so.

According to POSIX (http://pubs.opengroup.org/onlinepubs/009695399/functions/usleep.html):
````The useconds argument shall be less than one million.````
Make it so.
@mauser mauser merged commit 0f50fa0 into hydrogen-music:master Nov 30, 2016
@mauser
Copy link
Member

mauser commented Nov 30, 2016

Hi!
Thanks for the pull request! Just out of interest: How have you found this problem? Does it case an error on certain system?

@elpescado
Copy link
Contributor

elpescado commented Dec 1, 2016

I am ambivalent about this PR.

On one hand, it's clearly stated in POSIX docs that usleep argument should be less than 1 000 000.

On the other hand, I have never seen system that actually enforced that. Mac OS man page does not mention that limitation, and Linux one says:

ERRORS
(...)
EINVAL usec is not smaller than 1000000. (On systems where that is considered an error.)

Windows does not support it at all.

On the other other hand, if we are so picky about standards, according to POSIX 1.2001 usleep function is declared obsolete and in POSIX 1.2008 it is removed. That is also mentioned in Linux man page:

CONFORMING TO
4.3BSD, POSIX.1-2001. POSIX.1-2001 declares this function obsolete; use nanosleep(2) instead. POSIX.1-2008 removes the specification of usleep().

   On the original BSD implementation, and in glibc before version 2.2.2, the return type of this function is void.  The POSIX version returns int, and this is also the prototype used since glibc 2.2.2.

   Only the EINVAL error return is documented by SUSv2 and POSIX.1-2001.

So, finally, I propose, instead of calling usleep twice, use standard C++11 feature:

std::this_thread::sleep_for(std::chrono::seconds(1));

Or C++14:


using namespace std::literals;
std::this_thread::sleep_for(1s);

And get rid of that whole #ifdef:

#ifdef WIN32
#include <windows.h>
#define LOGGER_SLEEP Sleep( 100 )
#else
#include <unistd.h>
#define LOGGER_SLEEP do { usleep( 500000 ); usleep( 500000 ); } while (0)
#endif

@0-wiz-0
Copy link
Contributor Author

0-wiz-0 commented Dec 2, 2016

@mauser: It caused 100% CPU usage for a user, because the sleep was immediately terminated, so hydrogen was busylooping.
That's on NetBSD, where the one million limit is also documented in the man page:
http://netbsd.gw.com/cgi-bin/man-cgi?usleep++NetBSD-current

@trebmuh
Copy link
Member

trebmuh commented Dec 7, 2016

Written here: https://github.com/hydrogen-music/hydrogen/wiki/New-features-in-Hydrogen-1.0

mauser pushed a commit that referenced this pull request Feb 22, 2017
* Automation Paths: Create AutomationPath class

* Automation Paths: Add velocity Automation path to Song

* Automation Paths: add iterators, operator== and operator<< to AutomationPath

* Automation Paths: Serializer class tha loads/stores AutomationPath in XML document

* Automation Paths: Use velocity automation path in hydrogen player

* Automation Paths: Use velocity automation path in MIDI export

* Automation Paths: Load/Save automation paths in a song file

* Automation Paths: find and move points in paths

* Automation Paths: Add AutomationPAthView widget

* Automation Paths: Add AutomationPathView to SongEditorPanel

* Automation Paths: Display Velocity automation path in main window

* Automation Paths: Refine automation paths widget

* Automation Path: Set 1.5 as maximum velocity adjustment

* Automation Path: Correctly handle empty path

* Automation Paths: Modifying AutomationPath marks song as modified

* Automation Paths: Use velocity path only in song mode, not in pattern mode

* Automation Paths: AutomationPath::remove_point()

* Automation Paths: Add undo stack support for adding points

* AutomationPaths: Deleting points

* Automation Paths: Undo removing points

* Automation Paths: Make point moves undoable

* Automation Path: add doxygen comments

* Automation Paths: Add GPL header to source files

* Automation Paths: fix test

* Automation Paths: Qt5 compatibility

* Automation Paths: fix bug with adding points outside valid range

* Automation Path: remove unused <iostream> includes

* Automation Paths: tweak Automatio PAth presentation

* Improved spacing for translated labels

* Fix usleep argument (#444)

According to POSIX (http://pubs.opengroup.org/onlinepubs/009695399/functions/usleep.html):
````The useconds argument shall be less than one million.````
Make it so.

* Converted layouts to QFormLayout so widgets will resize

* Have export dialog size properly

* Correct behavior when cancelling window close

* typos + a few fixes for the French translation (#451)

* typos + a few fixes for the French translation

* one more typo

* Added padding along left side of dialog

* Velocity automation: add LCDCombo with description

* Automation Path: fix find() method not to look before first point
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.

4 participants