| Message ID | 20240303121454.16994-3-alx@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | Use terms consistently in function parameter names. | expand |
Alejandro Colomar wrote: > man2/clock_nanosleep.2 | 20 ++++++++++---------- > man2/nanosleep.2 | 12 ++++++------ The change to nanosleep.2 seems mostly fine. Except that the term "requested relative duration" (line 142) raises questions; what about changing that to "requested duration"? The change to clock_nanosleep.2 seems wrong. There are two cases (quoting the old text): If flags is 0, then the value specified in request is interpreted as an interval relative to the current value of the clock specified by clockid. If flags is TIMER_ABSTIME, then request is interpreted as an absolute time as measured by the clock, clockid. If request is less than or equal to the current value of the clock, then clock_nanosleep() returns immediately without suspending the calling thread. In the first case, the argument is a duration. In the second case, the argument is an absolute time point; it would be wrong and very confusing to denote it as "duration". Bruno
Hi Bruno, On Sun, Mar 03, 2024 at 01:45:37PM +0100, Bruno Haible wrote: > Alejandro Colomar wrote: > > man2/clock_nanosleep.2 | 20 ++++++++++---------- > > man2/nanosleep.2 | 12 ++++++------ > > The change to nanosleep.2 seems mostly fine. Except that the > term "requested relative duration" (line 142) raises questions; > what about changing that to "requested duration"? Yeah, I had doubts about that one. Probably I should drop 'relative'. > > The change to clock_nanosleep.2 seems wrong. There are two cases > (quoting the old text): > > If flags is 0, then the value specified in request is interpreted > as an interval relative to the current value of the clock > specified by clockid. > > If flags is TIMER_ABSTIME, then request is interpreted as an > absolute time as measured by the clock, clockid. If request is > less than or equal to the current value of the clock, then > clock_nanosleep() returns immediately without suspending the calling > thread. > > In the first case, the argument is a duration. In the second case, the > argument is an absolute time point; it would be wrong and very confusing > to denote it as "duration". Hmm, thanks! I guess we'll have to keep 'request' in clock_nanosleep(3) unless someone comes up with a better name. Elliott, you may want to partially revert that change in bionic. Have a lovely day! Alex
On Sun, Mar 3, 2024 at 4:55 AM Alejandro Colomar <alx@kernel.org> wrote: > > Hi Bruno, > > On Sun, Mar 03, 2024 at 01:45:37PM +0100, Bruno Haible wrote: > > Alejandro Colomar wrote: > > > man2/clock_nanosleep.2 | 20 ++++++++++---------- > > > man2/nanosleep.2 | 12 ++++++------ > > > > The change to nanosleep.2 seems mostly fine. Except that the > > term "requested relative duration" (line 142) raises questions; > > what about changing that to "requested duration"? > > Yeah, I had doubts about that one. Probably I should drop 'relative'. > > > > > The change to clock_nanosleep.2 seems wrong. There are two cases > > (quoting the old text): > > > > If flags is 0, then the value specified in request is interpreted > > as an interval relative to the current value of the clock > > specified by clockid. > > > > If flags is TIMER_ABSTIME, then request is interpreted as an > > absolute time as measured by the clock, clockid. If request is > > less than or equal to the current value of the clock, then > > clock_nanosleep() returns immediately without suspending the calling > > thread. > > > > In the first case, the argument is a duration. In the second case, the > > argument is an absolute time point; it would be wrong and very confusing > > to denote it as "duration". > > Hmm, thanks! I guess we'll have to keep 'request' in clock_nanosleep(3) > unless someone comes up with a better name. Elliott, you may want to > partially revert that change in bionic. thanks! https://android-review.googlesource.com/c/platform/bionic/+/2987070 changes to /** * [clock_nanosleep(2)](http://man7.org/linux/man-pages/man2/clock_nanosleep.2.html) * sleeps for the given time (or until the given time if the TIMER_ABSTIME flag * is used), as measured by the given clock. * * Returns 0 on success, and returns -1 and returns an error number on failure. * If the sleep was interrupted by a signal, the return value will be `EINTR` * and `remainder` will be the amount of time remaining. */ int clock_nanosleep(clockid_t __clock, int __flags, const struct timespec* _Nonnull __time, struct timespec* _Nullable __remainder); > Have a lovely day! > Alex > > -- > <https://www.alejandro-colomar.es/> > Looking for a remote C programming job at the moment.
Hi Elliott, On Mon, Mar 04, 2024 at 04:18:28PM -0800, enh wrote: > thanks! https://android-review.googlesource.com/c/platform/bionic/+/2987070 > changes to > > /** > * [clock_nanosleep(2)](http://man7.org/linux/man-pages/man2/clock_nanosleep.2.html) > * sleeps for the given time (or until the given time if the TIMER_ABSTIME flag > * is used), as measured by the given clock. > * > * Returns 0 on success, and returns -1 and returns an error number on failure. > * If the sleep was interrupted by a signal, the return value will be `EINTR` > * and `remainder` will be the amount of time remaining. > */ > int clock_nanosleep(clockid_t __clock, int __flags, const struct > timespec* _Nonnull __time, struct timespec* _Nullable __remainder); Hmmmm, that's the best name, meaningfully, I think. But I've been trying to avoid it. I don't like using names of standard functions in identifiers; it might confuse. As an alternative, I thought of 't'. What do you think? Have a lovely night! Alex
On Mon, Mar 4, 2024 at 4:34 PM Alejandro Colomar <alx@kernel.org> wrote: > > Hi Elliott, > > On Mon, Mar 04, 2024 at 04:18:28PM -0800, enh wrote: > > thanks! https://android-review.googlesource.com/c/platform/bionic/+/2987070 > > changes to > > > > /** > > * [clock_nanosleep(2)](http://man7.org/linux/man-pages/man2/clock_nanosleep.2.html) > > * sleeps for the given time (or until the given time if the TIMER_ABSTIME flag > > * is used), as measured by the given clock. > > * > > * Returns 0 on success, and returns -1 and returns an error number on failure. > > * If the sleep was interrupted by a signal, the return value will be `EINTR` > > * and `remainder` will be the amount of time remaining. > > */ > > int clock_nanosleep(clockid_t __clock, int __flags, const struct > > timespec* _Nonnull __time, struct timespec* _Nullable __remainder); > > Hmmmm, that's the best name, meaningfully, I think. But I've been > trying to avoid it. I don't like using names of standard functions in > identifiers; it might confuse. As an alternative, I thought of 't'. > What do you think? as you can see, i've taken the "the leading `__` means we get to trample whatever we like" approach :-) (we build bionic with hidden visibility and an explicit list of symbols for the linker to export, so we'd have to be trying quite hard to trip over ourselves.) > Have a lovely night! > > Alex > > -- > <https://www.alejandro-colomar.es/> > Looking for a remote C programming job at the moment.
On Mon, Mar 04, 2024 at 04:56:13PM -0800, enh wrote: > > > int clock_nanosleep(clockid_t __clock, int __flags, const struct > > > timespec* _Nonnull __time, struct timespec* _Nullable __remainder); > > > > Hmmmm, that's the best name, meaningfully, I think. But I've been > > trying to avoid it. I don't like using names of standard functions in > > identifiers; it might confuse. As an alternative, I thought of 't'. > > What do you think? > > as you can see, i've taken the "the leading `__` means we get to > trample whatever we like" approach :-) > > (we build bionic with hidden visibility and an explicit list of > symbols for the linker to export, so we'd have to be trying quite hard > to trip over ourselves.) Yeah, I was worried about the manual page :)
On Mon, Mar 4, 2024 at 5:11 PM Alejandro Colomar <alx@kernel.org> wrote: > > On Mon, Mar 04, 2024 at 04:56:13PM -0800, enh wrote: > > > > int clock_nanosleep(clockid_t __clock, int __flags, const struct > > > > timespec* _Nonnull __time, struct timespec* _Nullable __remainder); > > > > > > Hmmmm, that's the best name, meaningfully, I think. But I've been > > > trying to avoid it. I don't like using names of standard functions in > > > identifiers; it might confuse. As an alternative, I thought of 't'. > > > What do you think? > > > > as you can see, i've taken the "the leading `__` means we get to > > trample whatever we like" approach :-) > > > > (we build bionic with hidden visibility and an explicit list of > > symbols for the linker to export, so we'd have to be trying quite hard > > to trip over ourselves.) > > Yeah, I was worried about the manual page :) yeah, i think "t + extra text" makes sense there. i just try to be as brief as possible in the doc comments on the assumption that most readers will be seeing them in IDE pop-ups, and anyone who wants lots of text will click through to the man page anyway. and at that point they're your problem :-) > -- > <https://www.alejandro-colomar.es/> > Looking for a remote C programming job at the moment.
diff --git a/man2/clock_nanosleep.2 b/man2/clock_nanosleep.2 index 5bda50e18..0eedc1277 100644 --- a/man2/clock_nanosleep.2 +++ b/man2/clock_nanosleep.2 @@ -19,7 +19,7 @@ .SH SYNOPSIS .nf .P .BI "int clock_nanosleep(clockid_t " clockid ", int " flags , -.BI " const struct timespec *" request , +.BI " const struct timespec *" duration , .BI " struct timespec *_Nullable " remain ); .fi .P @@ -94,7 +94,7 @@ .SH DESCRIPTION If .I flags is 0, then the value specified in -.I request +.I duration is interpreted as an interval relative to the current value of the clock specified by .IR clockid . @@ -104,11 +104,11 @@ .SH DESCRIPTION is .BR TIMER_ABSTIME , then -.I request +.I duration is interpreted as an absolute time as measured by the clock, .IR clockid . If -.I request +.I duration is less than or equal to the current value of the clock, then .BR clock_nanosleep () @@ -117,7 +117,7 @@ .SH DESCRIPTION .BR clock_nanosleep () suspends the execution of the calling thread until either at least the time specified by -.I request +.I duration has elapsed, or a signal is delivered that causes a signal handler to be called or that terminates the process. @@ -138,7 +138,7 @@ .SH DESCRIPTION .BR clock_nanosleep () again and complete a (relative) sleep. .SH RETURN VALUE -On successfully sleeping for the requested interval, +On successfully sleeping for the requested duration, .BR clock_nanosleep () returns 0. If the call is interrupted by a signal handler or encounters an error, @@ -146,7 +146,7 @@ .SH RETURN VALUE .SH ERRORS .TP .B EFAULT -.I request +.I duration or .I remain specified an invalid address. @@ -179,8 +179,8 @@ .SH HISTORY Linux 2.6, glibc 2.1. .SH NOTES -If the interval specified in -.I request +If the +.I duration is not an exact multiple of the granularity underlying clock (see .BR time (7)), then the interval will be rounded up to the next multiple. @@ -216,7 +216,7 @@ .SH NOTES is .BR TIMER_ABSTIME . (An absolute sleep can be restarted using the same -.I request +.I duration argument.) .P POSIX.1 specifies that diff --git a/man2/nanosleep.2 b/man2/nanosleep.2 index a8d9f5a8a..6272c21e6 100644 --- a/man2/nanosleep.2 +++ b/man2/nanosleep.2 @@ -22,7 +22,7 @@ .SH SYNOPSIS .nf .B #include <time.h> .P -.BI "int nanosleep(const struct timespec *" req , +.BI "int nanosleep(const struct timespec *" duration , .BI " struct timespec *_Nullable " rem ); .fi .P @@ -39,7 +39,7 @@ .SH DESCRIPTION .BR nanosleep () suspends the execution of the calling thread until either at least the time specified in -.I *req +.I *duration has elapsed, or the delivery of a signal that triggers the invocation of a handler in the calling thread or that terminates the process. @@ -80,7 +80,7 @@ .SH DESCRIPTION and it makes the task of resuming a sleep that has been interrupted by a signal handler easier. .SH RETURN VALUE -On successfully sleeping for the requested interval, +On successfully sleeping for the requested duration, .BR nanosleep () returns 0. If the call is interrupted by a signal handler or encounters an error, @@ -139,7 +139,7 @@ .SH VERSIONS .BR nanosleep () function; ... Consequently, these time services shall expire when the requested relative -interval elapses, independently of the new or old value of the clock. +duration elapses, independently of the new or old value of the clock. .RE .SH STANDARDS POSIX.1-2008. @@ -158,8 +158,8 @@ .SH HISTORY This special extension was removed in Linux 2.5.39, and is thus not available in Linux 2.6.0 and later kernels. .SH NOTES -If the interval specified in -.I req +If the +.I duration is not an exact multiple of the granularity underlying clock (see .BR time (7)), then the interval will be rounded up to the next multiple.
It seems much more clear. Suggested-by: Elliott Hughes <enh@google.com> Cc: Stefan Puiu <stefan.puiu@gmail.com> Cc: Bruno Haible <bruno@clisp.org> Signed-off-by: Alejandro Colomar <alx@kernel.org> --- man2/clock_nanosleep.2 | 20 ++++++++++---------- man2/nanosleep.2 | 12 ++++++------ 2 files changed, 16 insertions(+), 16 deletions(-)