Re: [RFC PATCH glibc v2 1/1] hurd: Add CLOCK_MONOTONIC case in clock_gettime()

[Zhaoming Luo]

Thanks for reviewing.

On 1/3/25 6:52 PM, Sergey Bugaev wrote:
> On Fri, Jan 3, 2025 at 1:42 PM Zhaoming Luo <zhmingluo@163.com> wrote:
> > hurd: Add CLOCK_MONOTONIC case in clock_gettime()
>
> Nitpick: the prefix should be "mach:" instead of "hurd:".
>
> In theory, there could be other targets that also run on Mach, and
> sysdeps/mach/clock_gettime.c is only Mach-specific, not Hurd-specific
> (as the path also tells you, otherwise it'd go into
> sysdeps/mach/hurd/...). In practice of course the Hurd port is the
> only user of this file.
OK.
> > diff --git a/sysdeps/mach/clock_gettime.c b/sysdeps/mach/clock_gettime.c
> > index 6fffad39f5..546514b8ee 100644
> > --- a/sysdeps/mach/clock_gettime.c
> > +++ b/sysdeps/mach/clock_gettime.c
> > @@ -20,6 +20,7 @@
> >   #include <mach.h>
> >   #include <assert.h>
> >   #include <shlib-compat.h>
> > +#include <mach/mig_errors.h>
> >
> >   /* Get the current time of day, putting it into *TS.
> >      Returns 0 on success, -1 on errors.  */
> > @@ -31,6 +32,30 @@ __clock_gettime (clockid_t clock_id, struct timespec *ts)
> >
> >     switch (clock_id) {
> >
> > +    case CLOCK_MONOTONIC:
> > +/* If HAVE_HOST_GET_UPTIME64 is not defined, CLOCK_MONOTONIC will be equivalent
> > +   to CLOCK_REALTIME.  */
> > +#ifdef HAVE_HOST_GET_UPTIME64
> > +      {
> > +       time_value64_t tv;
> > +       err = __host_get_uptime64 (__mach_host_self (), &tv);
> > +
> > +       if (err)
> > +         {
> > +           if (err == MIG_BAD_ID)
> > +             {
> > +               /* Not supported by the running kernel.  */
> > +               __set_errno(EINVAL);
> > +             }
> > +           else
> > +             __set_errno(err);
> > +           return -1;
> > +         }
> > +       TIME_VALUE64_TO_TIMESPEC (&tv, ts);
> > +       return 0;
> > +      }
> > +#endif
>
> Ok. But please add the space before __set_errno and the opening paren.
OK. Forgot the space.
> > diff --git a/sysdeps/mach/configure.ac b/sysdeps/mach/configure.ac
> > index 6dfa2b3518..08e5fdefe9 100644
> > --- a/sysdeps/mach/configure.ac
> > +++ b/sysdeps/mach/configure.ac
> > @@ -92,6 +92,8 @@ fi
> >
> >   mach_RPC_CHECK(mach_host.defs, host_page_size,
> >                 HAVE_HOST_PAGE_SIZE)
> > +mach_RPC_CHECK(mach_host.defs, host_get_uptime64,
> > +              HAVE_HOST_GET_UPTIME64)
>
> This probably should've gone into gnumach.defs, but it's too late for
> that, isn't it.
I don't think it's too late. There is only one patch related to the 
change you suggest[0]. I'm just not sure why it should be in gnumach.defs.

[0]:https://git.savannah.gnu.org/cgit/hurd/gnumach.git/commit/?id=fc494bfe3fb6363e1077dc035eb119970d84a9d1
> >   mach_RPC_CHECK(gnumach.defs, thread_set_name,
> >                 HAVE_MACH_THREAD_SET_NAME)
> >   mach_RPC_CHECK(gnumach.defs, thread_get_name,
> > --
> > 2.45.2
>
> Sergey

--
Zhaoming Luo