[v2,1/6] support: Add xclock_gettime
Commit Message
* support/xclock_gettime.c (xclock_gettime): New file. Provide
clock_gettime wrapper for use in tests that fails the test rather
than returning failure.
* support/xtime.h: New file to declare xclock_gettime.
* support/Makefile: Add xclock_gettime.c.
* support/README: Mention xtime.h.
---
ChangeLog | 12 ++++++++++++
support/Makefile | 1 +
support/README | 1 +
support/xclock_gettime.c | 29 +++++++++++++++++++++++++++++
support/xtime.h | 33 +++++++++++++++++++++++++++++++++
5 files changed, 76 insertions(+)
create mode 100644 support/xclock_gettime.c
create mode 100644 support/xtime.h
Comments
On 07/04/2019 16:33, Mike Crowe wrote:
> * support/xclock_gettime.c (xclock_gettime): New file. Provide
> clock_gettime wrapper for use in tests that fails the test rather
> than returning failure.
>
> * support/xtime.h: New file to declare xclock_gettime.
>
> * support/Makefile: Add xclock_gettime.c.
>
> * support/README: Mention xtime.h.
LGTM with the change below.
> ---
> ChangeLog | 12 ++++++++++++
> support/Makefile | 1 +
> support/README | 1 +
> support/xclock_gettime.c | 29 +++++++++++++++++++++++++++++
> support/xtime.h | 33 +++++++++++++++++++++++++++++++++
> 5 files changed, 76 insertions(+)
> create mode 100644 support/xclock_gettime.c
> create mode 100644 support/xtime.h
>
> diff --git a/ChangeLog b/ChangeLog
> index 5ad8875..8c26806 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,15 @@
> +2019-04-06 Mike Crowe <mac@mcrowe.com>
> +
> + * support/xclock_gettime.c (xclock_gettime): New file. Provide
> + clock_gettime wrapper for use in tests that fails the test rather
> + than returning failure.
> +
> + * support/xtime.h: New file to declare xclock_gettime.
> +
> + * support/Makefile: Add xclock_gettime.c.
> +
> + * support/README: Mention xtime.h.
> +
> 2019-04-05 Anton Youdkevitch <anton.youdkevitch@bell-sw.com>
>
> * sysdeps/aarch64/multiarch/memcpy_thunderx2.S: Cleanup branching
> diff --git a/support/Makefile b/support/Makefile
> index f173565..1d37f70 100644
> --- a/support/Makefile
> +++ b/support/Makefile
> @@ -77,6 +77,7 @@ libsupport-routines = \
> xbind \
> xcalloc \
> xchroot \
> + xclock_gettime \
> xclose \
> xconnect \
> xcopy_file_range \
Ok.
> diff --git a/support/README b/support/README
> index 476cfcd..d82f472 100644
> --- a/support/README
> +++ b/support/README
> @@ -10,6 +10,7 @@ error. They are declared in these header files:
> * support.h
> * xsignal.h
> * xthread.h
> +* xtime.h
>
> In general, new wrappers should be added to support.h if possible.
> However, support.h must remain fully compatible with C90 and therefore
Ok.
> 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
> diff --git a/support/xtime.h b/support/xtime.h
> new file mode 100644
> index 0000000..68af1a5
> --- /dev/null
> +++ b/support/xtime.h
> @@ -0,0 +1,33 @@
> +/* Support functionality for using time.
> + 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/>. */
> +
> +#ifndef SUPPORT_TIME_H
> +#define SUPPORT_TIME_H
> +
> +#include <time.h>
> +
> +__BEGIN_DECLS
> +
> +/* The following functions call the corresponding libc functions and
> + terminate the process on error. */
> +
> +void xclock_gettime (clockid_t clock, struct timespec *ts);
> +
> +__END_DECLS
> +
> +#endif /* SUPPORT_TIME_H */
>
Ok.
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.
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.
@@ -1,3 +1,15 @@
+2019-04-06 Mike Crowe <mac@mcrowe.com>
+
+ * support/xclock_gettime.c (xclock_gettime): New file. Provide
+ clock_gettime wrapper for use in tests that fails the test rather
+ than returning failure.
+
+ * support/xtime.h: New file to declare xclock_gettime.
+
+ * support/Makefile: Add xclock_gettime.c.
+
+ * support/README: Mention xtime.h.
+
2019-04-05 Anton Youdkevitch <anton.youdkevitch@bell-sw.com>
* sysdeps/aarch64/multiarch/memcpy_thunderx2.S: Cleanup branching
@@ -77,6 +77,7 @@ libsupport-routines = \
xbind \
xcalloc \
xchroot \
+ xclock_gettime \
xclose \
xconnect \
xcopy_file_range \
@@ -10,6 +10,7 @@ error. They are declared in these header files:
* support.h
* xsignal.h
* xthread.h
+* xtime.h
In general, new wrappers should be added to support.h if possible.
However, support.h must remain fully compatible with C90 and therefore
new file mode 100644
@@ -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));
+}
new file mode 100644
@@ -0,0 +1,33 @@
+/* Support functionality for using time.
+ 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/>. */
+
+#ifndef SUPPORT_TIME_H
+#define SUPPORT_TIME_H
+
+#include <time.h>
+
+__BEGIN_DECLS
+
+/* The following functions call the corresponding libc functions and
+ terminate the process on error. */
+
+void xclock_gettime (clockid_t clock, struct timespec *ts);
+
+__END_DECLS
+
+#endif /* SUPPORT_TIME_H */