Re: [PATCH v2 1/6] support: Add xclock_gettime

[Mike Crowe <mac at mcrowe dot com>]

On Tuesday 23 April 2019 at 10:59:00 -0300, Adhemerval Zanella wrote:
> > diff --git a/support/xclock_gettime.c b/support/xclock_gettime.c
> > new file mode 100644
> > index 0000000..91464fe
> > --- /dev/null
> > +++ b/support/xclock_gettime.c
> > @@ -0,0 +1,29 @@
> > +/* clock_gettime with error checking.
> > +   Copyright (C) 2019 Free Software Foundation, Inc.
> > +   This file is part of the GNU C Library.
> > +
> > +   The GNU C Library is free software; you can redistribute it and/or
> > +   modify it under the terms of the GNU Lesser General Public
> > +   License as published by the Free Software Foundation; either
> > +   version 2.1 of the License, or (at your option) any later version.
> > +
> > +   The GNU C Library is distributed in the hope that it will be useful,
> > +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > +   Lesser General Public License for more details.
> > +
> > +   You should have received a copy of the GNU Lesser General Public
> > +   License along with the GNU C Library; if not, see
> > +   <http://www.gnu.org/licenses/>.  */
> > +
> > +#include <support/xtime.h>
> > +#include <support/xthread.h>
> > +
> > +
> > +void
> > +xclock_gettime (clockid_t clockid,
> > +                struct timespec *ts)
> > +{
> > +  xpthread_check_return
> > +    ("clock_gettime", clock_gettime (clockid, ts));
> > +}
> 
> xpthread_check_return uses the returned values as errno, while clock_gettime sets
> errno appropriately.  Just use other functions that set errno:
> 
>   int ret = clock_gettime (clockid, ts);
>   if (ret < 0)
>     FAIL_EXIT1 ("clock_gettime (\"%d\", {\"%ld\", \"%ld\"}): %m",
> 		clockid, (long int) ts.tv_sec, ts.tv_nsec);
>   return ret

Is it really worth outputting ts.tv_sec and ts.tv_nsec? If the call has
failed then they will just contain whatever was in them before the call -
which probably means uninitialised. Even worse, if clock_gettime fails with
EFAULT then attempting to read from ts will fault too.

I think I'd prefer just:

 FAIL_EXIT1 ("clock_gettime (%d): %m", clockid);

Are you happy with that?

Mike.