This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v2 1/6] support: Add xclock_gettime
On 25/04/2019 09:21, Mike Crowe wrote:
> 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.
Not really, it was just a suggestion in fact.
>
> I think I'd prefer just:
>
> FAIL_EXIT1 ("clock_gettime (%d): %m", clockid);
>
> Are you happy with that?
LGTM, thanks.