[LTP] [PATCH 1/2] SAFE_MACROS: Redirect to tst_brk_() early

[Jan Stancek]

----- Original Message -----
> From: "Cyril Hrubis" <chrubis@suse.cz>
> To: "Jan Stancek" <jstancek@redhat.com>
> Cc: ltp@lists.linux.it
> Sent: Monday, 13 February, 2017 9:09:29 AM
> Subject: Re: [LTP] [PATCH 1/2] SAFE_MACROS: Redirect to tst_brk_() early
> 
> Hi!
> > How about we provide different version of tst_brkm for library and
> > different
> > for tests? For library we obviously want to allow calls from both oldlib
> > and
> > newlib code, and redirect as needed. But for tests (users of library) we
> > would allow only oldlib tests to call tst_brkm.
> > 
> > So for tests tst_brkm_ stays the same - NORETURN, for library code, we
> > use either tst_brk_ or tst_brkm_, which means we'll need to fix any
> > potential warnings only in library.
> > 
> > This also forbids usage of tst_brkm from newlib tests, previously
> > it was OK if you didn't provide cleanup, now it would reject all
> > calls from newlib tests.
> 
> This sounds good. The only problem we could hit is some common code
> outside the lib/ directory used in both newlib and oldlib tests. Which
> was the original reason I put the redirection directly into the
> tst_brkm_() code...

Good point, I guess we'd have to do the same as for ltp/lib, that
is add -DLTPLIB and deal with warnings.

---

Or we don't make tst_brkm_ so strict, we keep it as it is,
but we find way to prevent newlib users only direct usage
of tst_brkm() macro:

diff --git a/include/old/safe_macros.h b/include/old/safe_macros.h
index 25001e806c12..96fb0c21876e 100644
--- a/include/old/safe_macros.h
+++ b/include/old/safe_macros.h
@@ -331,10 +331,10 @@ static inline int safe_setrlimit(const char *file, const int lineno,
 
 #define SAFE_IOCTL(cleanup_fn, fd, request, ...)             \
        ({int ret = ioctl(fd, request, __VA_ARGS__);         \
-         ret < 0 ?                                          \
-          tst_brkm(TBROK | TERRNO, cleanup_fn,              \
-                   "ioctl(%i,%s,...) failed", fd, #request) \
-        : ret;})
+         if (ret < 0)                                       \
+             tst_brkm(TBROK | TERRNO, cleanup_fn,           \
+                   "ioctl(%i,%s,...) failed", fd, #request); \
+         ret;})
 
 #endif /* __SAFE_MACROS_H__ */
 #endif /* __TEST_H__ */
diff --git a/include/old/test.h b/include/old/test.h
index d49256073741..93d0c360dfca 100644
--- a/include/old/test.h
+++ b/include/old/test.h
@@ -34,6 +34,10 @@
 #ifndef __TEST_H__
 #define __TEST_H__
 
+#ifdef TST_TEST_H__
+#error newlib already included
+#endif
+
 #include <stdio.h>
 #include <signal.h>
 #include <unistd.h>
@@ -153,9 +157,19 @@ void tst_resm_hexd_(const char *file, const int lineno, int ttype,
 void tst_brkm_(const char *file, const int lineno, int ttype,
        void (*func)(void), const char *arg_fmt, ...)
        __attribute__ ((format (printf, 5, 6))) LTP_ATTRIBUTE_NORETURN;
-#define tst_brkm(ttype, func, arg_fmt, ...) \
-       tst_brkm_(__FILE__, __LINE__, (ttype), (func), \
-                 (arg_fmt), ##__VA_ARGS__)
+
+#ifdef LTPLIB
+#include "ltp_priv.h"
+# define tst_brkm(flags, cleanup, fmt, ...) do { \
+               if (tst_test) \
+                       tst_brk_(__FILE__, __LINE__, flags, fmt, ##__VA_ARGS__); \
+               else \
+                       tst_brkm_(__FILE__, __LINE__, flags, cleanup, fmt, ##__VA_ARGS__); \
+               } while (0)
+#else
+# define tst_brkm(flags, cleanup, fmt, ...) \
+                       tst_brkm_(__FILE__, __LINE__, flags, cleanup, fmt, ##__VA_ARGS__)
+#endif
 
 void tst_require_root(void);
 void tst_exit(void) LTP_ATTRIBUTE_NORETURN;
diff --git a/include/tst_test.h b/include/tst_test.h
index 7dc371ccf095..463418ab2de2 100644
--- a/include/tst_test.h
+++ b/include/tst_test.h
@@ -18,6 +18,10 @@
 #ifndef TST_TEST_H__
 #define TST_TEST_H__
 
+#ifdef __TEST_H__
+#error oldlib already included
+#endif
+
 #include <unistd.h>
 
 #include "tst_common.h"
diff --git a/lib/Makefile b/lib/Makefile
index 663455ee31ad..6b3f6e1917fb 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -24,7 +24,7 @@ top_srcdir            ?= ..
 
 include $(top_srcdir)/include/mk/env_pre.mk
 
-CFLAGS                 += -I.
+CFLAGS                 += -I. -DLTPLIB
 
 ifneq ($(ANDROID),1)
 FILTER_OUT_DIRS                += android_libpthread android_librt


> We would also need to sprinkle the library code with returns as a part
> of the patch if I'm not mistaken.

Agreed, after removing NORETURN there's no way around that.

--

I keep thinking if we shouldn't do those ~30 trivial changes,
and have one simple tst_brkm() macro for both library and tests.

Regards,
Jan