Message ID | 82e30caf3e52201a31b6f736fbad64ffd069ea3f.1554665560.git-series.mac@mcrowe.com |
---|---|
State | New |
Headers | show |
Series | Add new helper functions+macros to libsupport and use them in some nptl tests | expand |
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.
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 \ 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 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)); +} 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 */