[LTP] [PATCH 1/2] lib: Add safe timerfd macros

[LTP] [PATCH 1/2] lib: Add safe timerfd macros

[Petr Vorel]

SAFE_TIMERFD_CREATE(), SAFE_TIMERFD_GETTIME() and SAFE_TIMERFD_SETTIME()

Added only to new C API.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Hi,

NOTE: ENOSYS is handled by ltp_syscall in lapi/timerfd.h.
+ I wonder include/tst_safe_timerfd.h and lapi/timerfd.h shouldn't be
merged into single file.

Kind regards,
Petr

 include/tst_safe_timerfd.h | 73 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 73 insertions(+)
 create mode 100644 include/tst_safe_timerfd.h

diff --git a/include/tst_safe_timerfd.h b/include/tst_safe_timerfd.h
new file mode 100644
index 000000000..4019527d6
--- /dev/null
+++ b/include/tst_safe_timerfd.h
@@ -0,0 +1,73 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 Petr Vorel <pvorel@suse.cz>
+ */
+
+#ifndef TST_SAFE_TIMERFD_H__
+#define TST_SAFE_TIMERFD_H__
+
+#include <errno.h>
+#include "lapi/timerfd.h"
+#include "tst_test.h"
+
+#define TTYPE (errno == ENOTSUP ? TCONF : TBROK)
+
+static inline int safe_timerfd_create(const char *file, const int lineno,
+				      int clockid, int flags)
+{
+	int fd;
+
+	fd = timerfd_create(clockid, flags);
+
+	if (fd < 0) {
+		tst_brk(TTYPE | TERRNO, "%s:%d: timerfd_create() failed",
+			file, lineno);
+	}
+
+	return fd;
+}
+
+static inline int safe_timerfd_gettime(const char *file, const int lineno,
+				int fd, struct itimerspec *curr_value)
+{
+	int rval;
+
+	rval = timerfd_gettime(fd, curr_value);
+	if (rval < 0) {
+		tst_brk(TTYPE | TERRNO, "%s:%d: timerfd_gettime() failed",
+			file, lineno);
+	}
+
+	return rval;
+}
+
+static inline int safe_timerfd_settime(const char *file, const int lineno,
+				int fd, int flags,
+				const struct itimerspec *new_value,
+				struct itimerspec *old_value)
+{
+	int rval;
+
+	if (tst_kvercmp(2, 6, 26) <= 0)
+		flags = 0;
+
+	rval = timerfd_settime(fd, flags, new_value, old_value);
+	if (rval < 0) {
+		tst_brk(TTYPE | TERRNO, "%s:%d: timerfd_settime() failed",
+			file, lineno);
+	}
+
+	return rval;
+}
+
+#define SAFE_TIMERFD_CREATE(clockid, flags)\
+	safe_timerfd_create(__FILE__, __LINE__, (clockid), (flags))
+
+#define SAFE_TIMERFD_GETTIME(fd, curr_value)\
+	safe_timerfd_gettime(__FILE__, __LINE__, (fd), (curr_value))
+
+#define SAFE_TIMERFD_SETTIME(fd, flags, new_value, old_value)\
+	safe_timerfd_settime(__FILE__, __LINE__, (fd), (flags), (new_value), \
+						 (old_value))
+
+#endif /* SAFE_TIMERFD_H__ */
-- 
2.25.1

[LTP] [PATCH 2/2] timerfd: Use safe macros

[Petr Vorel]

SAFE_TIMERFD_CREATE() and SAFE_TIMERFD_SETTIME()

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Safe functions will be used also in other timerfd related tests once
they're migrated to new C API.

Kind regards,
Petr

 testcases/kernel/syscalls/timerfd/timerfd01.c        |  9 +++------
 .../kernel/syscalls/timerfd/timerfd_settime02.c      | 12 ++----------
 2 files changed, 5 insertions(+), 16 deletions(-)

diff --git a/testcases/kernel/syscalls/timerfd/timerfd01.c b/testcases/kernel/syscalls/timerfd/timerfd01.c
index af19b4399..7bbde7283 100644
--- a/testcases/kernel/syscalls/timerfd/timerfd01.c
+++ b/testcases/kernel/syscalls/timerfd/timerfd01.c
@@ -19,7 +19,7 @@
 #include <poll.h>
 #include "tst_test.h"
 #include "tst_timer.h"
-#include "lapi/timerfd.h"
+#include "tst_safe_timerfd.h"
 
 static struct tcase {
 	int id;
@@ -47,8 +47,7 @@ static void settime(int tfd, struct itimerspec *tmr, int tflags,
 	tmr->it_value = tst_us_to_timespec(tvalue);
 	tmr->it_interval = tst_us_to_timespec(tinterval);
 
-	if (timerfd_settime(tfd, tflags, tmr, NULL))
-		tst_brk(TBROK | TERRNO, "timerfd_settime() failed");
+	SAFE_TIMERFD_SETTIME(tfd, tflags, tmr, NULL);
 }
 
 static void waittmr(int tfd, unsigned int exp_ticks)
@@ -87,9 +86,7 @@ static void run(unsigned int n)
 
 	tst_res(TINFO, "testing %s", clks->name);
 
-	tfd = timerfd_create(clks->id, 0);
-	if (tfd == -1)
-		tst_brk(TFAIL | TERRNO, "timerfd_create() failed");
+	tfd = SAFE_TIMERFD_CREATE(clks->id, 0);
 
 	tst_res(TINFO, "relative timer (100 ms)");
 	settime(tfd, &tmr, 0, 100 * 1000, 0);
diff --git a/testcases/kernel/syscalls/timerfd/timerfd_settime02.c b/testcases/kernel/syscalls/timerfd/timerfd_settime02.c
index 7fa26de62..0565802f4 100644
--- a/testcases/kernel/syscalls/timerfd/timerfd_settime02.c
+++ b/testcases/kernel/syscalls/timerfd/timerfd_settime02.c
@@ -15,7 +15,7 @@
  *  timerfd: Protect the might cancel mechanism proper
  */
 #include <unistd.h>
-#include <lapi/timerfd.h>
+#include "tst_safe_timerfd.h"
 #include "tst_test.h"
 #include "tst_fuzzy_sync.h"
 #include "tst_taint.h"
@@ -32,16 +32,8 @@ static struct tst_fzsync_pair fzsync_pair;
 
 static void setup(void)
 {
-	int ttype;
-
 	tst_taint_init(TST_TAINT_W | TST_TAINT_D);
-	errno = 0;
-	fd = timerfd_create(CLOCK_REALTIME, 0);
-
-	if (fd < 0) {
-		ttype = (errno == ENOTSUP ? TCONF : TBROK);
-		tst_brk(ttype | TERRNO, "Cannot create timer");
-	}
+	fd = SAFE_TIMERFD_CREATE(CLOCK_REALTIME, 0);
 
 	fzsync_pair.exec_loops = 1000000;
 	tst_fzsync_pair_init(&fzsync_pair);
-- 
2.25.1

[LTP] [PATCH 2/2] timerfd: Use safe macros

[Martin Doucha]

On 04. 03. 20 16:12, Petr Vorel wrote:
> SAFE_TIMERFD_CREATE() and SAFE_TIMERFD_SETTIME()
> 
> Signed-off-by: Petr Vorel <pvorel@suse.cz>

Reviewed-by: Martin Doucha <mdoucha@suse.cz>

-- 
Martin Doucha   mdoucha@suse.cz
QA Engineer for Software Maintenance
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic

[LTP] [PATCH 1/2] lib: Add safe timerfd macros

[Cyril Hrubis]

Hi!
> SAFE_TIMERFD_CREATE(), SAFE_TIMERFD_GETTIME() and SAFE_TIMERFD_SETTIME()
> 
> Added only to new C API.
> 
> Signed-off-by: Petr Vorel <pvorel@suse.cz>

I do have a patch that adds tst_clock_name() to the tst_safe_clocks.h I
will send it so that you can make use of it here as well.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 1/2] lib: Add safe timerfd macros

[Martin Doucha]

Hello

On 04. 03. 20 16:12, Petr Vorel wrote:
> SAFE_TIMERFD_CREATE(), SAFE_TIMERFD_GETTIME() and SAFE_TIMERFD_SETTIME()
> 
> Added only to new C API.
> 
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> Hi,
> 
> NOTE: ENOSYS is handled by ltp_syscall in lapi/timerfd.h.
> + I wonder include/tst_safe_timerfd.h and lapi/timerfd.h shouldn't be
> merged into single file.
> 
> Kind regards,
> Petr
> 
>  include/tst_safe_timerfd.h | 73 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 73 insertions(+)
>  create mode 100644 include/tst_safe_timerfd.h
> 
> diff --git a/include/tst_safe_timerfd.h b/include/tst_safe_timerfd.h
> new file mode 100644
> index 000000000..4019527d6
> --- /dev/null
> +++ b/include/tst_safe_timerfd.h
> @@ -0,0 +1,73 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2020 Petr Vorel <pvorel@suse.cz>
> + */
> +
> +#ifndef TST_SAFE_TIMERFD_H__
> +#define TST_SAFE_TIMERFD_H__
> +

Please split the following code off to a separate .c file:

> +#include <errno.h>
> +#include "lapi/timerfd.h"
> +#include "tst_test.h"
> +
> +#define TTYPE (errno == ENOTSUP ? TCONF : TBROK)
> +
> +static inline int safe_timerfd_create(const char *file, const int lineno,
> +				      int clockid, int flags)
> +{
> +	int fd;
> +

Don't forget to clear errno when you're not using the TEST() macro.

> +	fd = timerfd_create(clockid, flags);
> +
> +	if (fd < 0) {
> +		tst_brk(TTYPE | TERRNO, "%s:%d: timerfd_create() failed",
> +			file, lineno);
> +	}
> +
> +	return fd;
> +}
> +
> +static inline int safe_timerfd_gettime(const char *file, const int lineno,
> +				int fd, struct itimerspec *curr_value)
> +{
> +	int rval;
> +

Same here.

> +	rval = timerfd_gettime(fd, curr_value);
> +	if (rval < 0) {
> +		tst_brk(TTYPE | TERRNO, "%s:%d: timerfd_gettime() failed",
> +			file, lineno);
> +	}
> +
> +	return rval;
> +}
> +
> +static inline int safe_timerfd_settime(const char *file, const int lineno,
> +				int fd, int flags,
> +				const struct itimerspec *new_value,
> +				struct itimerspec *old_value)
> +{
> +	int rval;
> +

And here-ish.

> +	if (tst_kvercmp(2, 6, 26) <= 0)
> +		flags = 0;

I think tst_brk(TCONF) would be better here. Or at least tst_res(TWARN),
since resetting flags to 0 may render some tests useless.

> +
> +	rval = timerfd_settime(fd, flags, new_value, old_value);
> +	if (rval < 0) {
> +		tst_brk(TTYPE | TERRNO, "%s:%d: timerfd_settime() failed",
> +			file, lineno);
> +	}
> +
> +	return rval;
> +}

Split off up to here.

> +
> +#define SAFE_TIMERFD_CREATE(clockid, flags)\
> +	safe_timerfd_create(__FILE__, __LINE__, (clockid), (flags))
> +
> +#define SAFE_TIMERFD_GETTIME(fd, curr_value)\
> +	safe_timerfd_gettime(__FILE__, __LINE__, (fd), (curr_value))
> +
> +#define SAFE_TIMERFD_SETTIME(fd, flags, new_value, old_value)\
> +	safe_timerfd_settime(__FILE__, __LINE__, (fd), (flags), (new_value), \
> +						 (old_value))
> +
> +#endif /* SAFE_TIMERFD_H__ */
> 


-- 
Martin Doucha   mdoucha@suse.cz
QA Engineer for Software Maintenance
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic

[LTP] [PATCH 1/2] lib: Add safe timerfd macros

[Martin Doucha]

I forgot to add one more thing: making the return value checks more
pedantic would also be nice.

-- 
Martin Doucha   mdoucha@suse.cz
QA Engineer for Software Maintenance
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic

[LTP] [PATCH 1/2] lib: Add safe timerfd macros

[Petr Vorel]

Hi Martin,

> I forgot to add one more thing: making the return value checks more
> pedantic would also be nice.
I guess you mean if (rval == -1). Sure, I'll do that.

Kind regards,
Petr

[LTP] [PATCH 1/2] lib: Add safe timerfd macros

[Cyril Hrubis]

Hi!
> > I forgot to add one more thing: making the return value checks more
> > pedantic would also be nice.
> I guess you mean if (rval == -1). Sure, I'll do that.

Actually for the sake of SAFE_MACROS() it makes sense to fail unless we
got a valid result. So for > 0 for timerfd_create() and != 0 for
timerfd_settime() and timerfd_gettime().

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 1/2] lib: Add safe timerfd macros

[Petr Vorel]

Hi Cyril,

> > > I forgot to add one more thing: making the return value checks more
> > > pedantic would also be nice.
> > I guess you mean if (rval == -1). Sure, I'll do that.

> Actually for the sake of SAFE_MACROS() it makes sense to fail unless we
> got a valid result. So for > 0 for timerfd_create() 
I guess you mean fd < 0 here (no " > 0), as it's a file descriptor.
> and != 0 for
> timerfd_settime() and timerfd_gettime().

Kind regards,
Petr

[LTP] [PATCH 1/2] lib: Add safe timerfd macros

[Petr Vorel]

Hi Cyril, Martin,

> > Actually for the sake of SAFE_MACROS() it makes sense to fail unless we
> > got a valid result. So for > 0 for timerfd_create() 
> I guess you mean fd < 0 here (no " > 0), as it's a file descriptor.
Merged with this change.
Cyril, you can continue with your patchset.

Kind regards,
Petr

[LTP] [PATCH 1/2] lib: Add safe timerfd macros

[Cyril Hrubis]

Hi!
> Don't forget to clear errno when you're not using the TEST() macro.

Hmm, I'm not sure how useful is this. Generally the syscalls in libc
have single macro definition that is used everywhere to copy the error
from the errno variable. If that piece of code is buggy half of the test
in LTP would fail anyway.

...

> > +	if (tst_kvercmp(2, 6, 26) <= 0)
> > +		flags = 0;
> 
> I think tst_brk(TCONF) would be better here. Or at least tst_res(TWARN),
> since resetting flags to 0 may render some tests useless.

I think that it's completely wrong to put kernel version comparsion to
the safe_macros. If the test needs specific kernel version it should be
either put into the tst_test structure of handled in the test setup.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 1/2] lib: Add safe timerfd macros

[Cyril Hrubis]

Hi!
> > Don't forget to clear errno when you're not using the TEST() macro.
> 
> Hmm, I'm not sure how useful is this. Generally the syscalls in libc
> have single macro definition that is used everywhere to copy the error
> from the errno variable. If that piece of code is buggy half of the test
 ^
 to the

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 1/2] lib: Add safe timerfd macros

[Petr Vorel]

Hi Cyril,

> > Don't forget to clear errno when you're not using the TEST() macro.

> Hmm, I'm not sure how useful is this. Generally the syscalls in libc
> have single macro definition that is used everywhere to copy the error
> from the errno variable. If that piece of code is buggy half of the test
> in LTP would fail anyway.
That's exactly what I thought when I omit that.
+ we reset errno on fewer places than we don't.

> > > +	if (tst_kvercmp(2, 6, 26) <= 0)
> > > +		flags = 0;

> > I think tst_brk(TCONF) would be better here. Or at least tst_res(TWARN),
> > since resetting flags to 0 may render some tests useless.

> I think that it's completely wrong to put kernel version comparsion to
> the safe_macros. If the test needs specific kernel version it should be
> either put into the tst_test structure of handled in the test setup.
OK, I'll remove it.
For my previous state:
timerfd01.c (all)
timerfd_settime01.c (3th and 4rd test)
timerfd_settime02.c (not sure whether this old version is really affected by
CVE-2017-10661)

It means that 1st and 2nd test from timerfd_settime01.c could be tested on
2.6.25 and .26, but it won't due TCONF. But probably not many people will miss
it nowadays.

Kind regards,
Petr

[LTP] [PATCH 1/2] lib: Add safe timerfd macros

[Petr Vorel]

Hi Martin,

> Please split the following code off to a separate .c file:
OK, I'll do :). We had some discussions about it, I remember problems caused by
using lapi files in the library headers (tst_device.h), I'd consider ok having
just header tst_safe_timerfd.h unless it's loaded by library itself, but
somebody could do it later. And I also consider it as a cleaner solution
(hoping that linker will really drop unused symbols).

> > +#include <errno.h>
> > +#include "lapi/timerfd.h"
> > +#include "tst_test.h"
> > +
> > +#define TTYPE (errno == ENOTSUP ? TCONF : TBROK)
> > +
> > +static inline int safe_timerfd_create(const char *file, const int lineno,
> > +				      int clockid, int flags)
> > +{
> > +	int fd;
> > +

> Don't forget to clear errno when you're not using the TEST() macro.
Do you think it's necessary? I've noticed you clear errno here in original test,
but I'd expect errno is be set if (fd < 0).
But ok, it's done in some macros (tst_safe_macros.c, safe_macros.c)

> > +	if (tst_kvercmp(2, 6, 26) <= 0)
> > +		flags = 0;

> I think tst_brk(TCONF) would be better here. Or at least tst_res(TWARN),
> since resetting flags to 0 may render some tests useless.
Well, this is just to keep test working under 2.6.25 and .26. Obviously it
would not affect many users, but these would got broken these tests
timerfd01.c (all)
timerfd_settime01.c (3th and 4rd test)
timerfd_settime02.c (not sure whether this old version is really affected by
CVE-2017-10661)

break all tests). Agree that hiding it is not good.

> > +
> > +	rval = timerfd_settime(fd, flags, new_value, old_value);
> > +	if (rval < 0) {
> > +		tst_brk(TTYPE | TERRNO, "%s:%d: timerfd_settime() failed",
> > +			file, lineno);
> > +	}
> > +
> > +	return rval;
> > +}

> Split off up to here.
I guess you mean that macros should be kept in header (sure).

> > +
> > +#define SAFE_TIMERFD_CREATE(clockid, flags)\
> > +	safe_timerfd_create(__FILE__, __LINE__, (clockid), (flags))
> > +
> > +#define SAFE_TIMERFD_GETTIME(fd, curr_value)\
> > +	safe_timerfd_gettime(__FILE__, __LINE__, (fd), (curr_value))
> > +
> > +#define SAFE_TIMERFD_SETTIME(fd, flags, new_value, old_value)\
> > +	safe_timerfd_settime(__FILE__, __LINE__, (fd), (flags), (new_value), \
> > +						 (old_value))
> > +
> > +#endif /* SAFE_TIMERFD_H__ */

Kind regards,
Petr

[LTP] [PATCH 1/2] lib: Add safe timerfd macros

[Petr Vorel]

SAFE_TIMERFD_CREATE(), SAFE_TIMERFD_GETTIME() and SAFE_TIMERFD_SETTIME()

Added only to new C API.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Changes v1->v2:
* patch based on 1st and 2nd patch from Cyril's patchset "[01/12] lib:
* Move tst_clock_name() to tst_clock.c" (now posted only these 2)
https://patchwork.ozlabs.org/project/ltp/list/?series=162390&state=*
* Move implementation code to C file
* drop version check. BTW I was wrong, timerfd_create() requires flags
  to be zero (I had code in timerfd_settime()). And this is correctly
  handled in old tests (old tests which use non-zero flag already
  require correctly 2.6.27 instead of 2.6.25).
* check return for == -1 (instead of < 0)

NOTE: I decided ignore errno reset (and not use TEST()), as I agree with
Cyril ("Generally the syscalls in libc have single macro definition that
is used everywhere to copy the error from the errno variable. If that
piece of code is buggy half of the test in LTP would fail anyway.")

Kind regards,
Petr

 include/tst_safe_timerfd.h | 32 +++++++++++++++++++++
 lib/tst_safe_timerfd.c     | 59 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 91 insertions(+)
 create mode 100644 include/tst_safe_timerfd.h
 create mode 100644 lib/tst_safe_timerfd.c

diff --git a/include/tst_safe_timerfd.h b/include/tst_safe_timerfd.h
new file mode 100644
index 000000000..526f12838
--- /dev/null
+++ b/include/tst_safe_timerfd.h
@@ -0,0 +1,32 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 Petr Vorel <pvorel@suse.cz>
+ */
+
+#ifndef TST_SAFE_TIMERFD_H__
+#define TST_SAFE_TIMERFD_H__
+
+#include "lapi/timerfd.h"
+
+int safe_timerfd_create(const char *file, const int lineno,
+				      int clockid, int flags);
+
+#define SAFE_TIMERFD_CREATE(clockid, flags)\
+	safe_timerfd_create(__FILE__, __LINE__, (clockid), (flags))
+
+int safe_timerfd_gettime(const char *file, const int lineno,
+				int fd, struct itimerspec *curr_value);
+
+#define SAFE_TIMERFD_GETTIME(fd, curr_value)\
+	safe_timerfd_gettime(__FILE__, __LINE__, (fd), (curr_value))
+
+int safe_timerfd_settime(const char *file, const int lineno,
+				int fd, int flags,
+				const struct itimerspec *new_value,
+				struct itimerspec *old_value);
+
+#define SAFE_TIMERFD_SETTIME(fd, flags, new_value, old_value)\
+	safe_timerfd_settime(__FILE__, __LINE__, (fd), (flags), (new_value), \
+						 (old_value))
+
+#endif /* SAFE_TIMERFD_H__ */
diff --git a/lib/tst_safe_timerfd.c b/lib/tst_safe_timerfd.c
new file mode 100644
index 000000000..80de87ad3
--- /dev/null
+++ b/lib/tst_safe_timerfd.c
@@ -0,0 +1,59 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 Petr Vorel <pvorel@suse.cz>
+ */
+
+#include <errno.h>
+
+#include "tst_safe_timerfd.h"
+#include "lapi/timerfd.h"
+#include "tst_clocks.h"
+#define TST_NO_DEFAULT_MAIN
+#include "tst_test.h"
+
+#define TTYPE (errno == ENOTSUP ? TCONF : TBROK)
+
+int safe_timerfd_create(const char *file, const int lineno,
+				      int clockid, int flags)
+{
+	int fd;
+
+	fd = timerfd_create(clockid, flags);
+
+	if (fd == -1) {
+		tst_brk(TTYPE | TERRNO, "%s:%d timerfd_create(%s) failed",
+			file, lineno, tst_clock_name(clockid));
+	}
+
+	return fd;
+}
+
+int safe_timerfd_gettime(const char *file, const int lineno,
+				int fd, struct itimerspec *curr_value)
+{
+	int rval;
+
+	rval = timerfd_gettime(fd, curr_value);
+	if (rval == -1) {
+		tst_brk(TTYPE | TERRNO, "%s:%d timerfd_gettime() failed",
+			file, lineno);
+	}
+
+	return rval;
+}
+
+int safe_timerfd_settime(const char *file, const int lineno,
+				int fd, int flags,
+				const struct itimerspec *new_value,
+				struct itimerspec *old_value)
+{
+	int rval;
+
+	rval = timerfd_settime(fd, flags, new_value, old_value);
+	if (rval == -1) {
+		tst_brk(TTYPE | TERRNO, "%s:%d timerfd_settime() failed",
+			file, lineno);
+	}
+
+	return rval;
+}
-- 
2.25.1

[LTP] [PATCH 1/2] lib: Add safe timerfd macros

[Petr Vorel]

Hi,

I'm sorry I forget to add v2 in subject.

> Changes v1->v2:
> * patch based on 1st and 2nd patch from Cyril's patchset "[01/12] lib:
> * Move tst_clock_name() to tst_clock.c" (now posted only these 2)
> https://patchwork.ozlabs.org/project/ltp/list/?series=162390&state=*
And the reason was to make use of tst_clock_name() as Cyril suggested.

Kind regards,
Petr

[LTP] [PATCH 1/2] lib: Add safe timerfd macros

[Cyril Hrubis]

Hi!
>  include/tst_safe_timerfd.h | 32 +++++++++++++++++++++
>  lib/tst_safe_timerfd.c     | 59 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 91 insertions(+)
>  create mode 100644 include/tst_safe_timerfd.h
>  create mode 100644 lib/tst_safe_timerfd.c
> 
> diff --git a/include/tst_safe_timerfd.h b/include/tst_safe_timerfd.h
> new file mode 100644
> index 000000000..526f12838
> --- /dev/null
> +++ b/include/tst_safe_timerfd.h
> @@ -0,0 +1,32 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2020 Petr Vorel <pvorel@suse.cz>
> + */
> +
> +#ifndef TST_SAFE_TIMERFD_H__
> +#define TST_SAFE_TIMERFD_H__
> +
> +#include "lapi/timerfd.h"
> +
> +int safe_timerfd_create(const char *file, const int lineno,
> +				      int clockid, int flags);
> +
> +#define SAFE_TIMERFD_CREATE(clockid, flags)\
> +	safe_timerfd_create(__FILE__, __LINE__, (clockid), (flags))
> +
> +int safe_timerfd_gettime(const char *file, const int lineno,
> +				int fd, struct itimerspec *curr_value);
> +
> +#define SAFE_TIMERFD_GETTIME(fd, curr_value)\
> +	safe_timerfd_gettime(__FILE__, __LINE__, (fd), (curr_value))
> +
> +int safe_timerfd_settime(const char *file, const int lineno,
> +				int fd, int flags,
> +				const struct itimerspec *new_value,
> +				struct itimerspec *old_value);
> +
> +#define SAFE_TIMERFD_SETTIME(fd, flags, new_value, old_value)\
> +	safe_timerfd_settime(__FILE__, __LINE__, (fd), (flags), (new_value), \
> +						 (old_value))
> +
> +#endif /* SAFE_TIMERFD_H__ */
> diff --git a/lib/tst_safe_timerfd.c b/lib/tst_safe_timerfd.c
> new file mode 100644
> index 000000000..80de87ad3
> --- /dev/null
> +++ b/lib/tst_safe_timerfd.c
> @@ -0,0 +1,59 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2020 Petr Vorel <pvorel@suse.cz>
> + */
> +
> +#include <errno.h>

Errno is include in tst_test.h.

> +#include "tst_safe_timerfd.h"
> +#include "lapi/timerfd.h"
> +#include "tst_clocks.h"
> +#define TST_NO_DEFAULT_MAIN
> +#include "tst_test.h"
> +
> +#define TTYPE (errno == ENOTSUP ? TCONF : TBROK)
> +
> +int safe_timerfd_create(const char *file, const int lineno,
> +				      int clockid, int flags)
> +{
> +	int fd;
> +
> +	fd = timerfd_create(clockid, flags);
> +
> +	if (fd == -1) {

See the other email about the return values.

Other than that this looks good.

> +		tst_brk(TTYPE | TERRNO, "%s:%d timerfd_create(%s) failed",
> +			file, lineno, tst_clock_name(clockid));
> +	}
> +
> +	return fd;
> +}
> +
> +int safe_timerfd_gettime(const char *file, const int lineno,
> +				int fd, struct itimerspec *curr_value)
> +{
> +	int rval;
> +
> +	rval = timerfd_gettime(fd, curr_value);
> +	if (rval == -1) {
> +		tst_brk(TTYPE | TERRNO, "%s:%d timerfd_gettime() failed",
> +			file, lineno);
> +	}
> +
> +	return rval;
> +}
> +
> +int safe_timerfd_settime(const char *file, const int lineno,
> +				int fd, int flags,
> +				const struct itimerspec *new_value,
> +				struct itimerspec *old_value)
> +{
> +	int rval;
> +
> +	rval = timerfd_settime(fd, flags, new_value, old_value);
> +	if (rval == -1) {
> +		tst_brk(TTYPE | TERRNO, "%s:%d timerfd_settime() failed",
> +			file, lineno);
> +	}
> +
> +	return rval;
> +}
> -- 
> 2.25.1
> 

-- 
Cyril Hrubis
chrubis@suse.cz