Re: [PATCH] man/man3/timespec_get.3: Correct return value and clarify description

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, Feb 8, 2025 at 1:03 PM Alejandro Colomar <alx@xxxxxxxxxx> wrote:
>
> Hi Mark,
>
> On Sat, Feb 08, 2025 at 12:41:42PM -0800, Mark Harris wrote:
> > - 0, not -1, is returned for an unsupported time base or error
> >   (C23 7.29.2.6, 7.29.2.7; POSIX.1-2024 line 74358).
>
> LGTM.  C23 7.19.2.6p4 says
>
>         If the timespec_get function is successful it returns the
>         nonzero value base; otherwise, it returns zero.
>
> > - Clarify that any supported value of base is always nonzero (i.e.,
> >   there is no overlap between the two return value cases that may
> >   require errno or some other source to disambiguate)
> >   (C23 7.29.2.6, 7.29.2.7; POSIX.1-2024 line 74357).
>
> LGTM.  The paragraph quoted above confirms this.
>
> > - Clarify that timespec_getres(NULL, base) is a valid call to check
> >   whether the specified time base is supported (C23 7.29.2.7).
>
> LGTM.
>
> > - Clarify that the resolution for a particular time base is constant
> >   for the lifetime of the process (i.e., there is no need to retrieve
> >   it repeatedly) (C23 7.29.2.7).
>
> LGTM.  C23 7.19.2.7p2 says
>
>         For each supported base, multiple calls to the timespec_getres
>         function during the same program execution shall have identical
>         results.
>
> > - Calls to these functions are not technically equivalent to any
> >   clock_* function call; at least the return value will be different.
>
> It would be interesting to clarify if they are equivalent except for the
> return value.

The clock_* functions also specify that errno is set to certain values
on error, but the timespec_* functions do not guarantee this.  So
instead I just state that the time and resolution are the same.

>
> > - The ERRORS section is removed, because it states only what is true
> >   for every function that does not state otherwise (i.e., errno might
> >   be affected by underlying system calls).
>
> LGTM.
>
> > Signed-off-by: Mark Harris <mark.hsj@xxxxxxxxx>
>
> Please add appropriate tags:
>
>         Fixes: 7bda5119fe5e (2024-09-08; "timespec_get.3, timespec_getres.3: Add page and link page")
>         Cc: наб <nabijaczleweli@xxxxxxxxxxxxxxxxxx>
>
> > ---
> >  man/man3/timespec_get.3 | 62 ++++++++++++++++++++++++-----------------
> >  1 file changed, 36 insertions(+), 26 deletions(-)
> >
> > diff --git a/man/man3/timespec_get.3 b/man/man3/timespec_get.3
> > index 8c8d45d33..7993e138a 100644
> > --- a/man/man3/timespec_get.3
> > +++ b/man/man3/timespec_get.3
> > @@ -18,37 +18,47 @@ .SH SYNOPSIS
> >  .BI "int timespec_getres(struct timespec *" tp ", int " base );
> >  .fi
> >  .SH DESCRIPTION
> > -.I timespec_get(tp, TIME_UTC)
> > -is defined as
> > -.IR "clock_gettime(CLOCK_REALTIME, tp)" .
> > +The
> > +.BR timespec_get ()
> > +function stores the current time, based on the specified time base, in the
> > +.I struct timespec
>
> I would say
>
>         .BR timespec (3type)
>         structure

Ok.

>
> We usually say XXX structure in the manual pages, instead of struct XXX.
> Just for consistency, I'd follow that.
>
> > +pointed to by
> > +.IR res .
> >  .P
> > -.I timespec_getres(res, TIME_UTC)
> > -is equivalent to
> > -.IR "clock_getres(CLOCK_REALTIME, res)" .
> > +The
> > +.BR timespec_getres ()
> > +function stores the resolution of times retrieved by
> > +.BR timespec_get ()
> > +with the specified time base in the
> > +.I struct timespec
> > +pointed to by
> > +.IR tp ,
> > +if
> > +.I tp
> > +is non-NULL.
> > +For a particular time base,
> > +the resolution is constant for the lifetime of the calling process.
>
> LGTM.
>
> >  .P
> >  .B TIME_UTC
> > -is universally guaranteed to be a valid
> > -.IR base ,
> > -and is the only one supported under Linux.
> > -Some other systems support different time bases.
> > +is always a supported time base,
> > +and is the only time base supported on Linux.
> > +The time and resolution in this time base is the same as that retrieved by
>
> Maybe s/is/are/ and s/that/those/?

Ok.

>
> > +.I clock_gettime(CLOCK_REALTIME, res)
> > +and
> > +.IR "clock_getres(CLOCK_REALTIME, tp)" ,
>
> I'd use a non-breaking space:
>
>         .I clock_gettime(CLOCK_REALTIME,\~res)
>         and
>         .IR clock_getres(CLOCK_REALTIME,\~tp) ,

Ok.

>
> > +respectively.
> > +Other systems may support additional time bases.
>
> LGTM.
>
> >  .SH RETURN VALUE
> > -On success,
> > +.BR timespec_get ()
> > +returns the nonzero value
>
> I think I'd remove "value".  What do you think?

Ok, if you think that is sufficiently clear I will remove "value" and
s/represents/is/ (below).

>
> > +.I base
> > +if it represents a supported time base
> > +and the current time was successfully retrieved, or 0 otherwise.
>
> D'oh.  Someone designed another non-standard return value.  <facepalm/>

I agree that the return value is unusual, but of the dozens of
interfaces to get high resolution time we have finally stumbled onto
one that Linux, BSD, macOS, and Windows could all agree to support (at
least for TIME_UTC), and the unusual return value is less ugly than a
bunch of #ifdefs.


 - Mark

>
>
> Have a lovely night!
> Alex
>
> > +.P
> > +.BR timespec_getres ()
> > +returns the nonzero value
> >  .I base
> > -is returned.
> > -On error,
> > -\-1 is returned.
> > -.SH ERRORS
> > -Some C libraries
> > -.I may
> > -set
> > -.I errno
> > -to the same value as would be set by
> > -.BR clock_gettime (2)
> > -or
> > -.BR clock_getres (2).
> > -Neither C nor POSIX specify this,
> > -but they don't really indicate it shouldn't happen, either.
> > -Don't rely on this.
> > +if it represents a supported time base, or 0 otherwise.
> >  .SH ATTRIBUTES
> >  For an explanation of the terms used in this section, see
> >  .BR attributes (7).
> > --
> > 2.48.0
> >
>
> --
> <https://www.alejandro-colomar.es/>





[Index of Archives]     [Kernel Documentation]     [Netdev]     [Linux Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux