Re: sleep.3: miscellaneous cleanup and rewrites
On Tue, 27 Jul 2021 17:15:09 -0500, Scott Cheloha wrote: > We do it for other library functions where some part of the > functionality is implemented with a system call. In other manpages we > say something like: > > The > .Fn foo > function may fail and set > .Va errno > for any of the errors specified for the > .Fn bar 2 > routine. I think this is overkill. > Then again, there's really only one error. Maybe we should just > duplicate the information in sleep.3. > > What about this? > > The > .Fn sleep > function sets > .Va errno > to > .Dv EINTR > if it is interrupted by the delivery of a signal. Sure. - todd
Re: sleep.3: miscellaneous cleanup and rewrites
Hi Scott, Scott Cheloha wrote on Tue, Jul 27, 2021 at 05:15:09PM -0500: > On Mon, Jul 26, 2021 at 10:37:03AM -0600, Todd C. Miller wrote: >> On Mon, 26 Jul 2021 07:55:00 -0500, Scott Cheloha wrote: >>> Still wondering whether we need an Errors section to mention that >>> sleep(3) can set errno. >> POSIX doesn't define any errors. Of the errors returned by nanosleep, >> only EINTR is really possible. I guess we could save & restore >> errno in sleep.c but I don't see any implementations that actually >> do that. I only checked illumos, glibc and musl though. > We definitely don't want to save/restore. > > I meant more like a simple acknowledgement for the programmer. > Knowing what can set errno is helpful. > > We do it for other library functions where some part of the > functionality is implemented with a system call. In other manpages we > say something like: > > The > .Fn foo > function may fail and set > .Va errno > for any of the errors specified for the > .Fn bar 2 > routine. > > See, e.g., `man 3 nice` or `man 3 fopen`. > > Then again, there's really only one error. That would sound like overkill to me, especially since millert@ pointed out that only EINTR is relevant. > Maybe we should just > duplicate the information in sleep.3. > > What about this? > > The > .Fn sleep > function sets > .Va errno > to > .Dv EINTR > if it is interrupted by the delivery of a signal. I like that: it seems correct, complete, and concise. OK schwarze@ for the patch as a whole. Feel free to use or discard the two suggestions provided in-line below as you see fit. Yours, Ingo > Index: sleep.3 > === > RCS file: /cvs/src/lib/libc/gen/sleep.3,v > retrieving revision 1.16 > diff -u -p -r1.16 sleep.3 > --- sleep.3 8 Feb 2020 01:09:57 - 1.16 > +++ sleep.3 27 Jul 2021 22:13:22 - > @@ -32,7 +32,7 @@ > .Os > .Sh NAME > .Nm sleep > -.Nd suspend process execution for interval measured in seconds > +.Nd suspend execution for an interval of seconds > .Sh SYNOPSIS > .In unistd.h > .Ft unsigned int > @@ -40,41 +40,46 @@ > .Sh DESCRIPTION > The > .Fn sleep > -function suspends execution of the calling process until either > +function suspends execution until at least the given number of In nanosleep(2), you say ... suspends execution of the calling thread ... Would that be useful here, too? > .Fa seconds > -seconds have elapsed or a signal is delivered to the process and its > -action is to invoke a signal-catching function or to terminate the > -process. > -The suspension time may be longer than requested due to the > -scheduling of other activity by the system. > +have elapsed or an unmasked signal is delivered to the calling thread. > .Pp > -This function is implemented using > -.Xr nanosleep 2 > -by pausing for > -.Fa seconds > -seconds or until a signal occurs. > -Consequently, in this implementation, > -sleeping has no effect on the state of process timers, > -and there is no special handling for > -.Dv SIGALRM . > +This version of > +.Fn sleep > +is implemented with > +.Xr nanosleep 2 , > +so delivery of any unmasked signal will terminate the sleep early, > +even if > +.Dv SA_RESTART > +is set with > +.Xr sigaction 2 > +for the interrupting signal. > .Sh RETURN VALUES > -If the > +If > +.Fn sleep > +sleeps for the full count of > +.Fa seconds > +it returns 0. > +Otherwise, > +.Fn sleep > +returns the number of seconds remaining from the original request. > +.Sh ERRORS > +The > .Fn sleep > -function returns because the requested time has elapsed, the value > -returned will be zero. > -If the > -.Fn sleep > -function returns due to the delivery of a signal, the value returned > -will be the unslept amount (the request time minus the time actually > -slept) in seconds. > +function sets > +.Va errno > +to > +.Dv EINTR > +if it is interrupted by the delivery of a signal. > .Sh SEE ALSO > .Xr sleep 1 , > -.Xr nanosleep 2 > +.Xr nanosleep 2 , > +.Xr sigaction 2 > .Sh STANDARDS > The > .Fn sleep > function conforms to > -.St -p1003.1-90 . > +.St -p1003.1-2008 . I would consider it useful to add this sentence here: Setting .Va errno is an extension to that specification.
Re: sleep.3: miscellaneous cleanup and rewrites
On Mon, Jul 26, 2021 at 10:37:03AM -0600, Todd C. Miller wrote: > On Mon, 26 Jul 2021 07:55:00 -0500, Scott Cheloha wrote: > > > [...] > > > > Anyway, updated patch. No mention of SIGALRM or alarm(3) except in > > the History section. > > I think this is OK for now. > > > Still wondering whether we need an Errors section to mention that > > sleep(3) can set errno. > > POSIX doesn't define any errors. Of the errors returned by nanosleep, > only EINTR is really possible. I guess we could save & restore > errno in sleep.c but I don't see any implementations that actually > do that. I only checked illumos, glibc and musl though. We definitely don't want to save/restore. I meant more like a simple acknowledgement for the programmer. Knowing what can set errno is helpful. We do it for other library functions where some part of the functionality is implemented with a system call. In other manpages we say something like: The .Fn foo function may fail and set .Va errno for any of the errors specified for the .Fn bar 2 routine. See, e.g., `man 3 nice` or `man 3 fopen`. Then again, there's really only one error. Maybe we should just duplicate the information in sleep.3. What about this? The .Fn sleep function sets .Va errno to .Dv EINTR if it is interrupted by the delivery of a signal. Index: sleep.3 === RCS file: /cvs/src/lib/libc/gen/sleep.3,v retrieving revision 1.16 diff -u -p -r1.16 sleep.3 --- sleep.3 8 Feb 2020 01:09:57 - 1.16 +++ sleep.3 27 Jul 2021 22:13:22 - @@ -32,7 +32,7 @@ .Os .Sh NAME .Nm sleep -.Nd suspend process execution for interval measured in seconds +.Nd suspend execution for an interval of seconds .Sh SYNOPSIS .In unistd.h .Ft unsigned int @@ -40,41 +40,46 @@ .Sh DESCRIPTION The .Fn sleep -function suspends execution of the calling process until either +function suspends execution until at least the given number of .Fa seconds -seconds have elapsed or a signal is delivered to the process and its -action is to invoke a signal-catching function or to terminate the -process. -The suspension time may be longer than requested due to the -scheduling of other activity by the system. +have elapsed or an unmasked signal is delivered to the calling thread. .Pp -This function is implemented using -.Xr nanosleep 2 -by pausing for -.Fa seconds -seconds or until a signal occurs. -Consequently, in this implementation, -sleeping has no effect on the state of process timers, -and there is no special handling for -.Dv SIGALRM . +This version of +.Fn sleep +is implemented with +.Xr nanosleep 2 , +so delivery of any unmasked signal will terminate the sleep early, +even if +.Dv SA_RESTART +is set with +.Xr sigaction 2 +for the interrupting signal. .Sh RETURN VALUES -If the +If +.Fn sleep +sleeps for the full count of +.Fa seconds +it returns 0. +Otherwise, +.Fn sleep +returns the number of seconds remaining from the original request. +.Sh ERRORS +The .Fn sleep -function returns because the requested time has elapsed, the value -returned will be zero. -If the -.Fn sleep -function returns due to the delivery of a signal, the value returned -will be the unslept amount (the request time minus the time actually -slept) in seconds. +function sets +.Va errno +to +.Dv EINTR +if it is interrupted by the delivery of a signal. .Sh SEE ALSO .Xr sleep 1 , -.Xr nanosleep 2 +.Xr nanosleep 2 , +.Xr sigaction 2 .Sh STANDARDS The .Fn sleep function conforms to -.St -p1003.1-90 . +.St -p1003.1-2008 . .Sh HISTORY A .Fn sleep
Re: sleep.3: miscellaneous cleanup and rewrites
On Mon, 26 Jul 2021 07:55:00 -0500, Scott Cheloha wrote: > We don't know if it is still in the wild or not. > > There are lots of libc implementations. We can't account for all of > them. Right. We don't know who is still using a signal or itimer-based sleep. > > Being proscriptive in OpenBSD manual pages isn't going to stop someone > > from creating the precise problem you describe in some other body of > > code. Except, you are saying they don't create that problem. > > Right, we can't prevent it, which is why I wanted to say "beware". POSIX doesn't allow a SIGALRM-based implementation when threads are used anyway. A future version of POSIX will likely make a SIGALRM-based implementation non-conforming event for single-threaded processes. > > So why foam at the mouth over it? > > Because up until this very moment we referenced the historical > implementation approach in the manpage. The 4.4BSD man page had even more detail about this :-) > Anyway, updated patch. No mention of SIGALRM or alarm(3) except in > the History section. I think this is OK for now. > Still wondering whether we need an Errors section to mention that > sleep(3) can set errno. POSIX doesn't define any errors. Of the errors returned by nanosleep, only EINTR is really possible. I guess we could save & restore errno in sleep.c but I don't see any implementations that actually do that. I only checked illumos, glibc and musl though. - todd
Re: sleep.3: miscellaneous cleanup and rewrites
On Sun, Jul 25, 2021 at 07:41:47PM -0600, Theo de Raadt wrote: > Scott Cheloha wrote: > > > Given this, I want to tell the reader, roughly: > > > > "hey! it's plausible there is a SIGALRM-based sleep() implementation > > using still floating around out there in the wild. If you find one, > > you'll want to avoid using it because there are unfixable bugs in > > such an implementation. Maybe use nanosleep() instead. If you *do* > > use it, just know that it will behave differently from OpenBSD's > > sleep() in some corner cases." > > > > But if you really think there is no point in mentioning that, and > > others agree with you, then we won't mention it. > > I don't think the manual pages need to be proscriptive about a concern > which doesn't occur in the wild. We don't know if it is still in the wild or not. There are lots of libc implementations. We can't account for all of them. > Being proscriptive in OpenBSD manual pages isn't going to stop someone > from creating the precise problem you describe in some other body of > code. Except, you are saying they don't create that problem. Right, we can't prevent it, which is why I wanted to say "beware". > So why foam at the mouth over it? Because up until this very moment we referenced the historical implementation approach in the manpage. -- Anyway, updated patch. No mention of SIGALRM or alarm(3) except in the History section. Still wondering whether we need an Errors section to mention that sleep(3) can set errno. Otherwise I think this is about done. Index: sleep.3 === RCS file: /cvs/src/lib/libc/gen/sleep.3,v retrieving revision 1.16 diff -u -p -r1.16 sleep.3 --- sleep.3 8 Feb 2020 01:09:57 - 1.16 +++ sleep.3 26 Jul 2021 12:52:23 - @@ -32,7 +32,7 @@ .Os .Sh NAME .Nm sleep -.Nd suspend process execution for interval measured in seconds +.Nd suspend execution for an interval of seconds .Sh SYNOPSIS .In unistd.h .Ft unsigned int @@ -40,41 +40,38 @@ .Sh DESCRIPTION The .Fn sleep -function suspends execution of the calling process until either +function suspends execution until at least the given number of .Fa seconds -seconds have elapsed or a signal is delivered to the process and its -action is to invoke a signal-catching function or to terminate the -process. -The suspension time may be longer than requested due to the -scheduling of other activity by the system. +have elapsed or an unmasked signal is delivered to the calling thread. .Pp -This function is implemented using -.Xr nanosleep 2 -by pausing for -.Fa seconds -seconds or until a signal occurs. -Consequently, in this implementation, -sleeping has no effect on the state of process timers, -and there is no special handling for -.Dv SIGALRM . +This version of +.Fn sleep +is implemented with +.Xr nanosleep 2 , +so delivery of any unmasked signal will terminate the sleep early, +even if +.Dv SA_RESTART +is set with +.Xr sigaction 2 +for the interrupting signal. .Sh RETURN VALUES -If the +If .Fn sleep -function returns because the requested time has elapsed, the value -returned will be zero. -If the +sleeps for the full count of +.Fa seconds +it returns 0. +Otherwise, .Fn sleep -function returns due to the delivery of a signal, the value returned -will be the unslept amount (the request time minus the time actually -slept) in seconds. +returns the number of seconds remaining from the original request. .Sh SEE ALSO .Xr sleep 1 , -.Xr nanosleep 2 +.Xr nanosleep 2 , +.Xr sigaction 2 .Sh STANDARDS The .Fn sleep function conforms to -.St -p1003.1-90 . +.St -p1003.1-2008 . .Sh HISTORY A .Fn sleep
Re: sleep.3: miscellaneous cleanup and rewrites
Scott Cheloha wrote: > Given this, I want to tell the reader, roughly: > > "hey! it's plausible there is a SIGALRM-based sleep() implementation >using still floating around out there in the wild. If you find one, >you'll want to avoid using it because there are unfixable bugs in >such an implementation. Maybe use nanosleep() instead. If you *do* >use it, just know that it will behave differently from OpenBSD's >sleep() in some corner cases." > > But if you really think there is no point in mentioning that, and > others agree with you, then we won't mention it. I don't think the manual pages need to be proscriptive about a concern which doesn't occur in the wild. Being proscriptive in OpenBSD manual pages isn't going to stop someone from creating the precise problem you describe in some other body of code. Except, you are saying they don't create that problem. So why foam at the mouth over it?
Re: sleep.3: miscellaneous cleanup and rewrites
On Sun, Jul 25, 2021 at 05:21:19PM -0600, Theo de Raadt wrote: > Scott Cheloha wrote: > > > On Sun, Jul 25, 2021 at 01:35:19PM -0600, Theo de Raadt wrote: > > > This is a lot of fuss. > > > > > > How many bugs have you found relating to this issue? > > > > > > Let me guess: zero? > > > > Right, that's why I'm asking if we need to make a portability > > recommendation at all. > > > > We could also say something like: > > > > The standard permits sleep() implementations based on > > alarm(3). This implementation does not use alarm(3). > > > > ... or we could say nothing about SIGALRM. I don't know. > > Have you found any impacted implimentations? No. Can't find any current implementations. Older implementations, yes. > Why bother documenting a gotcha which noone has ever made? Not never. SIGALRM-based sleep() was commonplace until the end of the 90s and it's still permitted by the standard. OpenBSD had a SIGALRM-based sleep(3) up until 1997: http://cvsweb.openbsd.org/src/lib/libc/gen/sleep.c?rev=1.6=text/x-cvsweb-markup http://cvsweb.openbsd.org/src/lib/libc/gen/sleep.c?rev=1.7=text/x-cvsweb-markup Given this, I want to tell the reader, roughly: "hey! it's plausible there is a SIGALRM-based sleep() implementation using still floating around out there in the wild. If you find one, you'll want to avoid using it because there are unfixable bugs in such an implementation. Maybe use nanosleep() instead. If you *do* use it, just know that it will behave differently from OpenBSD's sleep() in some corner cases." But if you really think there is no point in mentioning that, and others agree with you, then we won't mention it.
Re: sleep.3: miscellaneous cleanup and rewrites
Scott Cheloha wrote: > On Sun, Jul 25, 2021 at 01:35:19PM -0600, Theo de Raadt wrote: > > This is a lot of fuss. > > > > How many bugs have you found relating to this issue? > > > > Let me guess: zero? > > Right, that's why I'm asking if we need to make a portability > recommendation at all. > > We could also say something like: > > The standard permits sleep() implementations based on > alarm(3). This implementation does not use alarm(3). > > ... or we could say nothing about SIGALRM. I don't know. Have you found any impacted implimentations? No. Why bother documenting a gotcha which noone has ever made?
Re: sleep.3: miscellaneous cleanup and rewrites
On Sun, Jul 25, 2021 at 01:35:19PM -0600, Theo de Raadt wrote: > This is a lot of fuss. > > How many bugs have you found relating to this issue? > > Let me guess: zero? Right, that's why I'm asking if we need to make a portability recommendation at all. We could also say something like: The standard permits sleep() implementations based on alarm(3). This implementation does not use alarm(3). ... or we could say nothing about SIGALRM. I don't know.
Re: sleep.3: miscellaneous cleanup and rewrites
On Sun, Jul 25, 2021 at 02:30:01PM -0500, Scott Cheloha wrote: > On Sun, Jul 25, 2021 at 08:15:34AM +0100, Jason McIntyre wrote: > > On Sat, Jul 24, 2021 at 10:39:49PM -0500, Scott Cheloha wrote: > > > Okay, the nanosleep.2 changes are committed, let's do sleep.3 next. > > > > hi. > > > > the changes read fine to me. only one comment: > > > > > [...] > > > > > > STANDARDS > > > > > > - Bump the relevant standard to POSIX.1-2001. That version > > > incorporates details about threads, which our version respects. > > > > > > For sake of completeness, I will note that SUSv2 mentions threads > > > too: > > > > > > https://pubs.opengroup.org/onlinepubs/7908799/xsh/sleep.html > > > > > > SUSv2 is older than POSIX.1-2001. Do we prefer the earlier > > > standard? > > > > > > > no, we tend to reference the latest spec. > > So, should I just say we conform to POSIX.1-2008? > yes, assuming that you're happy that it's conformant. jmc > There were no changes to the sleep() spec in that edition relevant to > our implementation. If sleep() is based on nanosleep() the spec > hasn't changed in any meaningful way since 2001. All the changes > since then are about the SIGALRM approach, which seems to be on its > way out. >
Re: sleep.3: miscellaneous cleanup and rewrites
This is a lot of fuss. How many bugs have you found relating to this issue? Let me guess: zero? Scott Cheloha wrote: > On Sun, Jul 25, 2021 at 08:15:34AM +0100, Jason McIntyre wrote: > > On Sat, Jul 24, 2021 at 10:39:49PM -0500, Scott Cheloha wrote: > > > Okay, the nanosleep.2 changes are committed, let's do sleep.3 next. > > > > hi. > > > > the changes read fine to me. only one comment: > > > > > [...] > > > > > > STANDARDS > > > > > > - Bump the relevant standard to POSIX.1-2001. That version > > > incorporates details about threads, which our version respects. > > > > > > For sake of completeness, I will note that SUSv2 mentions threads > > > too: > > > > > > https://pubs.opengroup.org/onlinepubs/7908799/xsh/sleep.html > > > > > > SUSv2 is older than POSIX.1-2001. Do we prefer the earlier > > > standard? > > > > > > > no, we tend to reference the latest spec. > > So, should I just say we conform to POSIX.1-2008? > > There were no changes to the sleep() spec in that edition relevant to > our implementation. If sleep() is based on nanosleep() the spec > hasn't changed in any meaningful way since 2001. All the changes > since then are about the SIGALRM approach, which seems to be on its > way out. >
Re: sleep.3: miscellaneous cleanup and rewrites
On Sun, Jul 25, 2021 at 08:15:34AM +0100, Jason McIntyre wrote: > On Sat, Jul 24, 2021 at 10:39:49PM -0500, Scott Cheloha wrote: > > Okay, the nanosleep.2 changes are committed, let's do sleep.3 next. > > hi. > > the changes read fine to me. only one comment: > > > [...] > > > > STANDARDS > > > > - Bump the relevant standard to POSIX.1-2001. That version > > incorporates details about threads, which our version respects. > > > > For sake of completeness, I will note that SUSv2 mentions threads > > too: > > > > https://pubs.opengroup.org/onlinepubs/7908799/xsh/sleep.html > > > > SUSv2 is older than POSIX.1-2001. Do we prefer the earlier > > standard? > > > > no, we tend to reference the latest spec. So, should I just say we conform to POSIX.1-2008? There were no changes to the sleep() spec in that edition relevant to our implementation. If sleep() is based on nanosleep() the spec hasn't changed in any meaningful way since 2001. All the changes since then are about the SIGALRM approach, which seems to be on its way out.
Re: sleep.3: miscellaneous cleanup and rewrites
On Sat, Jul 24, 2021 at 10:39:49PM -0500, Scott Cheloha wrote: > Okay, the nanosleep.2 changes are committed, let's do sleep.3 next. > hi. the changes read fine to me. only one comment: > Here's a changelist by section. I have some questions in there at end > of sections where I'm unsure about something. > > NAME > > - This is clunky. Tighten it up. > > - It's also wrong. Remove reference to the process: sleep suspends > the calling thread. > > DESCRIPTION > > - Again, it suspends the thread, not the process. > > - If we borrow language/style from nanosleep.2 we can condense this > first paragraph into a single sentence. > > - In the second paragraph I don't think we need to talk about SIGALRM > explicitly. It isn't relevant to our implementation. > > I think such comments belong in the Standards section. See below. > > - Probably worth mentioning the sole implication of an implementation > based on nanosleep(2) here instead of telling the reader to schlep > off to that page to get the details there. > > I'm not positive about cutting the discussion of timers and SIGALARM > here. My thinking is: here in 2021, the fact that our sleep() doesn't > touch the process timer or the signal mask doesn't seem significant > enough to warrant inclusion. I wonder if this detail confuses some > readers ("why are they bringing this up?"). > > RETURN VALUES > > - Tighten this up. > > - Prefer a literal "0" for an integral return value. > > Should we mention that sleep(3) can set errno? > > If so, do we need to mention how to check whether it was interrupted > by a signal? It's similar to other libc syscall wrappers: > > errno = 0; > sleep(...); > if (errno != 0) > warn("sleep"); > > No, you can't just check if the return value is greater than 0. If we > get a signal but the scheduler doesn't choose to run us until after > the timeout has elapsed we'll get 0 back but errno will be set. No > way to avoid this race. > > ERRORS > > Is this section missing? Maybe just a nod to nanosleep(2), like: > > The sleep() function may fail for any of the reasons > given in nanosleep(2). > > Something else? > > SEE ALSO > > - Add sigaction(2). We mention SA_RESTART and signals in the > Description. > > STANDARDS > > - Bump the relevant standard to POSIX.1-2001. That version > incorporates details about threads, which our version respects. > > For sake of completeness, I will note that SUSv2 mentions threads > too: > > https://pubs.opengroup.org/onlinepubs/7908799/xsh/sleep.html > > SUSv2 is older than POSIX.1-2001. Do we prefer the earlier > standard? > no, we tend to reference the latest spec. jmc > - Mention the two possible implementations. Mention that they > are not the same and might behave differently. Offer a > solution for portable applications. > > For the portability paragraph I'm just spitballing. No idea what the > right language is here or what recommendations are appropriate. > > In general, the nanosleep() spec is simple and the sleep() spec is > complex due to some historical mishaps. From a spec perspective, > nanosleep(2) is the safer bet because you know what you're getting, > warts and all. > > However: > > I honestly haven't looked very deeply into the specific behavioral > differences between the two approaches, I'm just taking POSIX at their > word. We have an old-school implementation in CVS: > > http://cvsweb.openbsd.org/src/lib/libc/gen/sleep.c?rev=1.6=text/x-cvsweb-markup > > ... at a glance I can imagine how it might behave differently from the > current approach: > > http://cvsweb.openbsd.org/src/lib/libc/gen/sleep.c?rev=1.13=text/x-cvsweb-markup > > I also don't know where you would find an alarm(3)-based approach > anymore. I caveat my recommendation with the "although the > alarm(3)-based approach is increasingly rare" bit to acknowledge this, > though still I feel like we ought to say something, just in case. > > HISTORY > > - I cannot find a sleep() system call in Research UNIX v2. > > I can find it in v3: > > > https://github.com/dspinellis/unix-history-repo/blob/Research-V3/man/man2/sleep.2 > > So, change ".At v2" to ".At v3". > > I must admit I have a hard time tracking the changes in the Research > UNIX code. Shit moves around *constantly*. So sleep() might indeed > be in v2 and I'm just not seeing it. > > -- > > Index: sleep.3 > === > RCS file: /cvs/src/lib/libc/gen/sleep.3,v > retrieving revision 1.16 > diff -u -p -r1.16 sleep.3 > --- sleep.3 8 Feb 2020 01:09:57 - 1.16 > +++ sleep.3 25 Jul 2021 03:27:27 - > @@ -32,7 +32,7 @@ > .Os > .Sh NAME > .Nm sleep > -.Nd suspend process execution for interval measured in seconds > +.Nd suspend execution for a count of seconds > .Sh SYNOPSIS > .In unistd.h > .Ft unsigned int > @@ -40,46 +40,57 @@ > .Sh DESCRIPTION >
Re: sleep.3: miscellaneous cleanup and rewrites
On Sat, Jul 24, 2021 at 10:39:49PM -0500, Scott Cheloha wrote: > Okay, the nanosleep.2 changes are committed, let's do sleep.3 next. > > Here's a changelist by section. I have some questions in there at end > of sections where I'm unsure about something. > > NAME > > - This is clunky. Tighten it up. > > - It's also wrong. Remove reference to the process: sleep suspends > the calling thread. > > DESCRIPTION > > - Again, it suspends the thread, not the process. > > - If we borrow language/style from nanosleep.2 we can condense this > first paragraph into a single sentence. > > - In the second paragraph I don't think we need to talk about SIGALRM > explicitly. It isn't relevant to our implementation. > > I think such comments belong in the Standards section. See below. > > - Probably worth mentioning the sole implication of an implementation > based on nanosleep(2) here instead of telling the reader to schlep > off to that page to get the details there. > > I'm not positive about cutting the discussion of timers and SIGALARM > here. My thinking is: here in 2021, the fact that our sleep() doesn't > touch the process timer or the signal mask doesn't seem significant > enough to warrant inclusion. I wonder if this detail confuses some > readers ("why are they bringing this up?"). > > RETURN VALUES > > - Tighten this up. > > - Prefer a literal "0" for an integral return value. > > Should we mention that sleep(3) can set errno? > > If so, do we need to mention how to check whether it was interrupted > by a signal? It's similar to other libc syscall wrappers: > > errno = 0; > sleep(...); > if (errno != 0) > warn("sleep"); > > No, you can't just check if the return value is greater than 0. If we > get a signal but the scheduler doesn't choose to run us until after > the timeout has elapsed we'll get 0 back but errno will be set. No > way to avoid this race. > > ERRORS > > Is this section missing? Maybe just a nod to nanosleep(2), like: > > The sleep() function may fail for any of the reasons > given in nanosleep(2). > > Something else? > > SEE ALSO > > - Add sigaction(2). We mention SA_RESTART and signals in the > Description. > > STANDARDS > > - Bump the relevant standard to POSIX.1-2001. That version > incorporates details about threads, which our version respects. > > For sake of completeness, I will note that SUSv2 mentions threads > too: > > https://pubs.opengroup.org/onlinepubs/7908799/xsh/sleep.html > > SUSv2 is older than POSIX.1-2001. Do we prefer the earlier > standard? > > - Mention the two possible implementations. Mention that they > are not the same and might behave differently. Offer a > solution for portable applications. > > For the portability paragraph I'm just spitballing. No idea what the > right language is here or what recommendations are appropriate. > > In general, the nanosleep() spec is simple and the sleep() spec is > complex due to some historical mishaps. From a spec perspective, > nanosleep(2) is the safer bet because you know what you're getting, > warts and all. > > However: > > I honestly haven't looked very deeply into the specific behavioral > differences between the two approaches, I'm just taking POSIX at their > word. We have an old-school implementation in CVS: > > http://cvsweb.openbsd.org/src/lib/libc/gen/sleep.c?rev=1.6=text/x-cvsweb-markup > > ... at a glance I can imagine how it might behave differently from the > current approach: > > http://cvsweb.openbsd.org/src/lib/libc/gen/sleep.c?rev=1.13=text/x-cvsweb-markup > > I also don't know where you would find an alarm(3)-based approach > anymore. I caveat my recommendation with the "although the > alarm(3)-based approach is increasingly rare" bit to acknowledge this, > though still I feel like we ought to say something, just in case. > > HISTORY > > - I cannot find a sleep() system call in Research UNIX v2. > > I can find it in v3: > > > https://github.com/dspinellis/unix-history-repo/blob/Research-V3/man/man2/sleep.2 > > So, change ".At v2" to ".At v3". > > I must admit I have a hard time tracking the changes in the Research > UNIX code. Shit moves around *constantly*. So sleep() might indeed > be in v2 and I'm just not seeing it. See the "Combined Tables of Contents" in Doug McIlroy's "A Research UNIX Reader". 2. System calls Edition Title Purpose 1 2 3 4 5 6 7 8 9 . + + + + + . . . sleep delay execution [alarm] And indeed it is documented in https://www.tuhs.org/Archive/Distributions/Research/Dennis_v2/v2man.pdf > > -- > > Index: sleep.3 > === > RCS file: /cvs/src/lib/libc/gen/sleep.3,v > retrieving revision 1.16 > diff -u -p -r1.16 sleep.3 > --- sleep.3 8 Feb 2020 01:09:57 - 1.16 > +++ sleep.3 25 Jul 2021 03:27:27 - > @@ -32,7 +32,7 @@ > .Os > .Sh NAME > .Nm sleep > -.Nd