[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1409469353.4476.269.camel@eris.loria.fr>
Date: Sun, 31 Aug 2014 09:15:53 +0200
From: Jens Gustedt <jens.gustedt@...ia.fr>
To: musl@...ts.openwall.com
Subject: Re: [PATCH 2/8] additions to src/time
Am Samstag, den 30.08.2014, 20:13 -0400 schrieb Rich Felker:
> On Sat, Aug 30, 2014 at 08:46:35PM +0200, Jens Gustedt wrote:
> > This adds two functions, thrd_sleep and timespec_get. Both have easy
> > functional equivalences in POSIX, but these have subtle differences in
> > the handling of errors.
> >
> > thrd_sleep forces concrete numerical values as error return
> >
> > timespec_get has a call interface that is incompatible with POSIX because
> > it has a bogus coding for its clock constants, and also this clock
> > constants must be returned in case of success. For the moment we only
> > implement one single clock, TIME_UTC, and map this to
> > CLOCK_REALTIME. This is the clock that we later need to measure time for
> > the timedlock and timedwait.
> >
> > Also, other than for clock_gettime, C doesn't specify touching errno for
> > timespec_get. Because CLOCK_REALTIME can be obtained very efficiently
> > through VDSO and messing with errno is clearly the wrong step to go, we
> > try to avoid this.
> > ---
> > src/time/thrd_sleep.c | 26 ++++++++++++++++++++++++++
> > src/time/timespec_get.c | 31 +++++++++++++++++++++++++++++++
> > 2 files changed, 57 insertions(+)
> > create mode 100644 src/time/thrd_sleep.c
> > create mode 100644 src/time/timespec_get.c
> >
> > diff --git a/src/time/thrd_sleep.c b/src/time/thrd_sleep.c
> > new file mode 100644
> > index 0000000..3dbfe47
> > --- /dev/null
> > +++ b/src/time/thrd_sleep.c
> > @@ -0,0 +1,26 @@
> > +#include <time.h>
> > +#include <errno.h>
> > +#include "syscall.h"
> > +#include "libc.h"
> > +#include "threads.h"
> > +
> > +/* Roughly __syscall already returns the right thing: 0 if all went
> > + well or a negative error indication, otherwise. */
> > +int thrd_sleep(const struct timespec *req, struct timespec *rem)
> > +{
> > + int ret = __syscall(SYS_nanosleep, req, rem);
> > + switch (ret) {
> > + case 0:
> > + return 0;
> > + /* error described by POSIX: */
> > + /* EINTR The nanosleep() function was interrupted by a signal. */
> > + /* The C11 wording is: */
> > + /* -1 if it has been interrupted by a signal */
>
> Purely coding style, but for musl we generally do multi-line comments
> in this form:
>
> /* line 1
> * line 2
> * line 3 */
I don't remeber having seen that in the coding style recommendations :)
I'll fix those that I notice, and keep that in mind for future use.
> Using // would be a second-choice alternative. I don't like the
> right-alignment of */ with spaces because it will look like a mess to
> someone reading it with a non-fixed-width font.
>
> > diff --git a/src/time/timespec_get.c b/src/time/timespec_get.c
> > new file mode 100644
> > index 0000000..20080a0
> > --- /dev/null
> > +++ b/src/time/timespec_get.c
> > @@ -0,0 +1,31 @@
> > +#include <time.h>
> > +#include "syscall.h"
> > +#include "atomic.h"
> > +
> > +static int syscall_clock_gettime(clockid_t clk, struct timespec *ts)
> > +{
> > + return __syscall(SYS_clock_gettime, clk, ts);
> > +}
> > +
> > +void *__vdsosym(const char *, const char *);
> > +
> > +/* There is no other implemented value than TIME_UTC, all other values
> > + are considered erroneous. */
> > +int timespec_get(struct timespec * ts, int base)
> > +{
> > + if (base != TIME_UTC) return 0;
> > + int ret;
> > +#ifdef VDSO_CGT_SYM
> > + static int (*cgt)(clockid_t, struct timespec *);
> > + if (!cgt) {
> > + void *f = __vdsosym(VDSO_CGT_VER, VDSO_CGT_SYM);
> > + if (!f) f = (void *)syscall_clock_gettime;
> > + a_cas_p(&cgt, 0, f);
> > + }
> > + /* The vdso variants never fail, and thus never set errno. */
> > + ret = cgt(CLOCK_REALTIME, ts);
> > +#else
> > + ret = syscall_clock_gettime(CLOCK_REALTIME, ts);
> > +#endif
> > + return ret < 0 ? 0 : base;
>
> I'd rather not duplicate this code but just call __clock_gettime.
ok
> Unlike __clock_gettime, your duplicate code does not handle
> pre-clock_gettime kernels and will return an error on them.
yeah, somehow I missed the gettimeofday syscall.
Would it be acceptable by any chance to have a common core for these
two functions that doesn't set errno? It would perhaps be appropriate
to use such a function in other places of musl, too.
Jens
--
:: INRIA Nancy Grand Est ::: AlGorille ::: ICube/ICPS :::
:: ::::::::::::::: office Strasbourg : +33 368854536 ::
:: :::::::::::::::::::::: gsm France : +33 651400183 ::
:: ::::::::::::::: gsm international : +49 15737185122 ::
:: http://icube-icps.unistra.fr/index.php/Jens_Gustedt ::
Download attachment "signature.asc" of type "application/pgp-signature" (199 bytes)
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
