[LTP] [PATCH 1/4] tst_test.h, test.h: Add mutual exclusion guards

[LTP] [PATCH 1/4] tst_test.h, test.h: Add mutual exclusion guards

[Cyril Hrubis]

These headers cannot be used at the same time, this makes sure they
aren't.

Signed-off-by: Jan Stancek <jstancek@redhat.com>
Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 include/old/test.h | 4 ++++
 include/tst_test.h | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/include/old/test.h b/include/old/test.h
index 209dc5b..2cb7b98 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 tst_test.h already included
+#endif /* TST_TEST_H__ */
+
 #include <stdio.h>
 #include <signal.h>
 #include <unistd.h>
diff --git a/include/tst_test.h b/include/tst_test.h
index 7dc371c..d8cc806 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 test.h already included
+#endif /* __TEST_H__ */
+
 #include <unistd.h>
 
 #include "tst_common.h"
-- 
2.10.2

[LTP] [PATCH 2/4] lib: Redirect to tst_brk_() early

[Cyril Hrubis]

This is first patch of a patchset that would allow us to use SAFE_MACROS() in
newlib testcases. After the patch we redirect to tst_brk_() in case of newlib
tests directly in the test library code.

This is needed since the tst_brkm_() from the old library is marked as
attribute ((noreturn)) and because of that the return address is not
saved on stack and hence we cannot return from the function. Removing
the atrribute is not an option either since that generates ~1000
"control reaches end of non-void function" warnings.

This commit redefines tst_brkm in test.h for the test library code so that we
branch depending on if we are running oldlib/newlib testcase and call
corresponding tst_brk_() or tst_brkm_() function directly instead of branching
in the tst_brkm_() code.

We also had to add a few returns to various places in the test library so that
we exit corresponding function in a case that tst_brkm() actually returned.

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 include/old/safe_macros.h    | 12 ++++++------
 include/old/test.h           | 17 ++++++++++++++---
 lib/Makefile                 |  2 +-
 lib/safe_file_ops.c          | 34 ++++++++++++++++++++++++++++------
 lib/safe_macros.c            |  9 +++++++++
 lib/safe_net.c               |  2 ++
 lib/self_exec.c              | 12 ++++++++++--
 lib/tst_checkpoint.c         |  2 ++
 lib/tst_device.c             |  9 +++++++--
 lib/tst_fs_has_free.c        |  1 +
 lib/tst_fs_link_count.c      | 12 ++++++++++--
 lib/tst_fs_type.c            |  1 +
 lib/tst_kernel.c             |  4 +++-
 lib/tst_mkfs.c               |  4 ++++
 lib/tst_module.c             |  3 +++
 lib/tst_net.c                | 17 +++++++++++++----
 lib/tst_path_has_mnt_flags.c |  2 ++
 lib/tst_resource.c           |  5 ++++-
 lib/tst_run_cmd.c            |  8 +++++++-
 lib/tst_timer.c              |  1 +
 lib/tst_tmpdir.c             | 25 +++++++++++++++++++------
 lib/tst_virt.c               |  2 ++
 22 files changed, 149 insertions(+), 35 deletions(-)

diff --git a/include/old/safe_macros.h b/include/old/safe_macros.h
index 25001e8..e778d30 100644
--- a/include/old/safe_macros.h
+++ b/include/old/safe_macros.h
@@ -329,12 +329,12 @@ static inline int safe_setrlimit(const char *file, const int lineno,
 	safe_readdir(__FILE__, __LINE__, (cleanup_fn), (dirp))
 
 
-#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;})
+#define SAFE_IOCTL(cleanup_fn, fd, request, ...)                   \
+	({int ret = ioctl(fd, request, __VA_ARGS__);               \
+	  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 2cb7b98..b36764d 100644
--- a/include/old/test.h
+++ b/include/old/test.h
@@ -157,9 +157,20 @@ 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, ...) do { \
+		tst_brkm_(__FILE__, __LINE__, flags, cleanup, fmt, ##__VA_ARGS__); \
+	} while (0)
+#endif
 
 void tst_require_root(void);
 void tst_exit(void) LTP_ATTRIBUTE_NORETURN;
diff --git a/lib/Makefile b/lib/Makefile
index de45bef..bfbdf25 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
diff --git a/lib/safe_file_ops.c b/lib/safe_file_ops.c
index 01f64ed..b576cb9 100644
--- a/lib/safe_file_ops.c
+++ b/lib/safe_file_ops.c
@@ -142,6 +142,7 @@ void safe_file_scanf(const char *file, const int lineno,
 		tst_brkm(TBROK | TERRNO, cleanup_fn,
 			 "Failed to open FILE '%s' for reading at %s:%d",
 			 path, file, lineno);
+		return;
 	}
 
 	exp_convs = count_scanf_conversions(fmt);
@@ -154,18 +155,21 @@ void safe_file_scanf(const char *file, const int lineno,
 		tst_brkm(TBROK, cleanup_fn,
 			 "The FILE '%s' ended prematurely at %s:%d",
 			 path, file, lineno);
+		return;
 	}
 
 	if (ret != exp_convs) {
 		tst_brkm(TBROK, cleanup_fn,
 			 "Expected %i conversions got %i FILE '%s' at %s:%d",
 			 exp_convs, ret, path, file, lineno);
+		return;
 	}
 
 	if (fclose(f)) {
 		tst_brkm(TBROK | TERRNO, cleanup_fn,
 			 "Failed to close FILE '%s' at %s:%d",
 			 path, file, lineno);
+		return;
 	}
 }
 
@@ -188,6 +192,7 @@ int file_lines_scanf(const char *file, const int lineno,
 	if (!fmt) {
 		tst_brkm(TBROK, cleanup_fn, "pattern is NULL, %s:%d",
 			file, lineno);
+		return 1;
 	}
 
 	fp = fopen(path, "r");
@@ -195,6 +200,7 @@ int file_lines_scanf(const char *file, const int lineno,
 		tst_brkm(TBROK | TERRNO, cleanup_fn,
 			"Failed to open FILE '%s' for reading at %s:%d",
 			path, file, lineno);
+		return 1;
 	}
 
 	arg_count = count_scanf_conversions(fmt);
@@ -209,9 +215,11 @@ int file_lines_scanf(const char *file, const int lineno,
 	}
 	fclose(fp);
 
-	if (strict && ret != arg_count)
+	if (strict && ret != arg_count) {
 		tst_brkm(TBROK, cleanup_fn, "Expected %i conversions got %i"
 			" at %s:%d", arg_count, ret, file, lineno);
+		return 1;
+	}
 
 	return !(ret == arg_count);
 }
@@ -273,6 +281,7 @@ void safe_file_printf(const char *file, const int lineno,
 		tst_brkm(TBROK | TERRNO, cleanup_fn,
 			 "Failed to open FILE '%s' for writing at %s:%d",
 			 path, file, lineno);
+		return;
 	}
 
 	va_start(va, fmt);
@@ -281,6 +290,7 @@ void safe_file_printf(const char *file, const int lineno,
 		tst_brkm(TBROK, cleanup_fn,
 			 "Failed to print to FILE '%s' at %s:%d",
 			 path, file, lineno);
+		return;
 	}
 
 	va_end(va);
@@ -289,6 +299,7 @@ void safe_file_printf(const char *file, const int lineno,
 		tst_brkm(TBROK | TERRNO, cleanup_fn,
 			 "Failed to close FILE '%s' at %s:%d",
 			 path, file, lineno);
+		return;
 	}
 }
 
@@ -342,23 +353,29 @@ void safe_touch(const char *file, const int lineno,
 	defmode = S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH;
 
 	ret = open(pathname, O_CREAT | O_WRONLY, defmode);
-	if (ret == -1)
+	if (ret == -1) {
 		tst_brkm(TBROK | TERRNO, cleanup_fn,
 			"Failed to open file '%s' at %s:%d",
 			pathname, file, lineno);
+		return;
+	}
 
 	ret = close(ret);
-	if (ret == -1)
+	if (ret == -1) {
 		tst_brkm(TBROK | TERRNO, cleanup_fn,
 			"Failed to close file '%s' at %s:%d",
 			pathname, file, lineno);
+		return;
+	}
 
 	if (mode != 0) {
 		ret = chmod(pathname, mode);
-		if (ret == -1)
+		if (ret == -1) {
 			tst_brkm(TBROK | TERRNO, cleanup_fn,
 				"Failed to chmod file '%s' at %s:%d",
 				pathname, file, lineno);
+			return;
+		}
 	}
 
 
@@ -372,16 +389,21 @@ void safe_touch(const char *file, const int lineno,
 		struct timeval cotimes[2];
 
 		ret = stat(pathname, &sb);
-		if (ret == -1)
+		if (ret == -1) {
 			tst_brkm(TBROK | TERRNO, cleanup_fn,
 				"Failed to stat file '%s' at %s:%d",
 				pathname, file, lineno);
+			return;
+		}
 
 		ret = gettimeofday(cotimes, NULL);
-		if (ret == -1)
+		if (ret == -1) {
 			tst_brkm(TBROK | TERRNO, cleanup_fn,
 				"Failed to gettimeofday()@%s:%d",
 				file, lineno);
+			return;
+		}
+
 		cotimes[1] = cotimes[0];
 
 		set_time(cotimes, times,
diff --git a/lib/safe_macros.c b/lib/safe_macros.c
index d696a0b..42e7d98 100644
--- a/lib/safe_macros.c
+++ b/lib/safe_macros.c
@@ -461,17 +461,20 @@ long safe_strtol(const char *file, const int lineno,
 	    || (errno != 0 && rval == 0)) {
 		tst_brkm(TBROK | TERRNO, cleanup_fn,
 			 "%s:%d: strtol(%s) failed", file, lineno, str);
+		return rval;
 	}
 
 	if (endptr == str || (*endptr != '\0' && *endptr != '\n')) {
 		tst_brkm(TBROK, cleanup_fn,
 			 "%s:%d: strtol(%s): Invalid value", file, lineno, str);
+		return 0;
 	}
 
 	if (rval > max || rval < min) {
 		tst_brkm(TBROK, cleanup_fn,
 			 "%s:%d: strtol(%s): %ld is out of range %ld - %ld",
 			 file, lineno, str, rval, min, max);
+		return 0;
 	}
 
 	return rval;
@@ -491,17 +494,20 @@ unsigned long safe_strtoul(const char *file, const int lineno,
 	    || (errno != 0 && rval == 0)) {
 		tst_brkm(TBROK | TERRNO, cleanup_fn,
 			 "%s:%d: strtoul(%s) failed", file, lineno, str);
+		return rval;
 	}
 
 	if (rval > max || rval < min) {
 		tst_brkm(TBROK, cleanup_fn,
 			 "%s:%d: strtoul(%s): %lu is out of range %lu - %lu",
 			 file, lineno, str, rval, min, max);
+		return 0;
 	}
 
 	if (endptr == str || (*endptr != '\0' && *endptr != '\n')) {
 		tst_brkm(TBROK, cleanup_fn,
 			 "Invalid value: '%s'@%s:%d", str, file, lineno);
+		return 0;
 	}
 
 	return rval;
@@ -797,6 +803,7 @@ int safe_setxattr(const char *file, const int lineno, const char *path,
 			tst_brkm(TCONF, NULL,
 				 "%s:%d: no xattr support in fs or mounted "
 				 "without user_xattr option", file, lineno);
+			return rval;
 		}
 
 		tst_brkm(TBROK | TERRNO, NULL, "%s:%d: setxattr() failed",
@@ -818,6 +825,7 @@ int safe_lsetxattr(const char *file, const int lineno, const char *path,
 			tst_brkm(TCONF, NULL,
 				 "%s:%d: no xattr support in fs or mounted "
 				 "without user_xattr option", file, lineno);
+			return rval;
 		}
 
 		tst_brkm(TBROK | TERRNO, NULL, "%s:%d: lsetxattr() failed",
@@ -839,6 +847,7 @@ int safe_fsetxattr(const char *file, const int lineno, int fd, const char *name,
 			tst_brkm(TCONF, NULL,
 				 "%s:%d: no xattr support in fs or mounted "
 				 "without user_xattr option", file, lineno);
+			return rval;
 		}
 
 		tst_brkm(TBROK | TERRNO, NULL, "%s:%d: fsetxattr() failed",
diff --git a/lib/safe_net.c b/lib/safe_net.c
index cae77b5..2c929ad 100644
--- a/lib/safe_net.c
+++ b/lib/safe_net.c
@@ -114,6 +114,7 @@ int safe_bind(const char *file, const int lineno, void (cleanup_fn)(void),
 				 socket, tst_sock_addr(address, address_len,
 						       buf, sizeof(buf)),
 				 address_len);
+			return -1;
 		}
 
 		if ((i + 1) % 10 == 0) {
@@ -129,6 +130,7 @@ int safe_bind(const char *file, const int lineno, void (cleanup_fn)(void),
 		 lineno, socket,
 		 tst_sock_addr(address, address_len, buf, sizeof(buf)),
 		 address_len);
+	return -1;
 }
 
 int safe_listen(const char *file, const int lineno, void (cleanup_fn)(void),
diff --git a/lib/self_exec.c b/lib/self_exec.c
index 94f85b6..c287ce9 100644
--- a/lib/self_exec.c
+++ b/lib/self_exec.c
@@ -87,6 +87,7 @@ void maybe_run_child(void (*child) (), const char *fmt, ...)
 		if (strlen(child_dir) == 0) {
 			tst_brkm(TBROK, NULL,
 				 "Could not get directory from -C option");
+			return;
 		}
 
 		va_start(ap, fmt);
@@ -96,6 +97,7 @@ void maybe_run_child(void (*child) (), const char *fmt, ...)
 			if (!tok || strlen(tok) == 0) {
 				tst_brkm(TBROK, NULL,
 					 "Invalid argument to -C option");
+				return;
 			}
 
 			switch (*p) {
@@ -105,6 +107,7 @@ void maybe_run_child(void (*child) (), const char *fmt, ...)
 				if (*endptr != '\0') {
 					tst_brkm(TBROK, NULL,
 						 "Invalid argument to -C option");
+					return;
 				}
 				*iptr = i;
 				break;
@@ -114,6 +117,7 @@ void maybe_run_child(void (*child) (), const char *fmt, ...)
 				if (*endptr != '\0') {
 					tst_brkm(TBROK, NULL,
 						 "Invalid argument to -C option");
+					return;
 				}
 				if (j != i) {
 					va_end(ap);
@@ -126,6 +130,7 @@ void maybe_run_child(void (*child) (), const char *fmt, ...)
 				if (!strncpy(s, tok, strlen(tok) + 1)) {
 					tst_brkm(TBROK, NULL,
 						 "Could not strncpy for -C option");
+					return;
 				}
 				break;
 			case 'S':
@@ -134,21 +139,24 @@ void maybe_run_child(void (*child) (), const char *fmt, ...)
 				if (!*sptr) {
 					tst_brkm(TBROK, NULL,
 						 "Could not strdup for -C option");
+					return;
 				}
 				break;
 			default:
 				tst_brkm(TBROK, NULL,
 					 "Format string option %c not implemented",
 					 *p);
-				break;
+				return;
 			}
 		}
 
 		va_end(ap);
 		free(args);
-		if (chdir(child_dir) < 0)
+		if (chdir(child_dir) < 0) {
 			tst_brkm(TBROK, NULL,
 				 "Could not change to %s for child", child_dir);
+			return;
+		}
 
 		(*child) ();
 		tst_resm(TWARN, "Child function returned unexpectedly");
diff --git a/lib/tst_checkpoint.c b/lib/tst_checkpoint.c
index a23bd02..e8e35ae 100644
--- a/lib/tst_checkpoint.c
+++ b/lib/tst_checkpoint.c
@@ -46,6 +46,7 @@ void tst_checkpoint_init(const char *file, const int lineno,
 		tst_brkm(TBROK, cleanup_fn,
 		         "%s: %d checkopoints allready initialized",
 		         file, lineno);
+		return;
 	}
 
 	/*
@@ -63,6 +64,7 @@ void tst_checkpoint_init(const char *file, const int lineno,
 		tst_brkm(TBROK, cleanup_fn,
 		         "%s:%d You have to create test temporary directory "
 		         "first (call tst_tmpdir())", file, lineno);
+		return;
 	}
 
 	page_size = getpagesize();
diff --git a/lib/tst_device.c b/lib/tst_device.c
index 4cefd60..5c91a54 100644
--- a/lib/tst_device.c
+++ b/lib/tst_device.c
@@ -278,18 +278,23 @@ const char *tst_acquire_device_(void (cleanup_fn)(void), unsigned int size)
 {
 	const char *device;
 
-	if (device_acquired)
+	if (device_acquired) {
 		tst_brkm(TBROK, cleanup_fn, "Device allready acquired");
+		return NULL;
+	}
 
 	if (!tst_tmpdir_created()) {
 		tst_brkm(TBROK, cleanup_fn,
 		         "Cannot acquire device without tmpdir() created");
+		return NULL;
 	}
 
 	device = tst_acquire_device__(size);
 
-	if (!device)
+	if (!device) {
 		tst_brkm(TBROK, cleanup_fn, "Failed to acquire device");
+		return NULL;
+	}
 
 	return device;
 }
diff --git a/lib/tst_fs_has_free.c b/lib/tst_fs_has_free.c
index 5c10376..e82dfa8 100644
--- a/lib/tst_fs_has_free.c
+++ b/lib/tst_fs_has_free.c
@@ -34,6 +34,7 @@ int tst_fs_has_free_(void (*cleanup)(void), const char *path,
 	if (statfs(path, &sf)) {
 		tst_brkm(TBROK | TERRNO, cleanup,
 			 "tst_fs_has_free: failed to statfs(%s)", path);
+		return 0;
 	}
 
 	if ((uint64_t)sf.f_bavail * sf.f_bsize >= (uint64_t)size * mult)
diff --git a/lib/tst_fs_link_count.c b/lib/tst_fs_link_count.c
index ca0c77e..860510d 100644
--- a/lib/tst_fs_link_count.c
+++ b/lib/tst_fs_link_count.c
@@ -50,8 +50,10 @@ int tst_fs_fill_hardlinks_(void (*cleanup) (void), const char *dir)
 		SAFE_MKDIR(cleanup, dir, 0744);
 
 	SAFE_STAT(cleanup, dir, &s);
-	if (!S_ISDIR(s.st_mode))
+	if (!S_ISDIR(s.st_mode)) {
 		tst_brkm(TBROK, cleanup, "%s is not directory", dir);
+		return 0;
+	}
 
 	sprintf(base_filename, "%s/testfile0", dir);
 	SAFE_TOUCH(cleanup, base_filename, 0644, NULL);
@@ -70,6 +72,7 @@ int tst_fs_fill_hardlinks_(void (*cleanup) (void), const char *dir)
 					 "hard links for %s have %i, should be"
 					 " %d", base_filename,
 					 (int)s.st_nlink, i);
+				return 0;
 			} else {
 				tst_resm(TINFO, "the maximum number of hard "
 					 "links to %s is hit: %d",
@@ -85,6 +88,7 @@ int tst_fs_fill_hardlinks_(void (*cleanup) (void), const char *dir)
 			tst_brkm(TBROK, cleanup, "link(%s, %s) failed "
 				 "unexpectedly: %s", base_filename,
 				 link_filename, strerror(errno));
+			return 0;
 		}
 	}
 
@@ -110,8 +114,10 @@ int tst_fs_fill_subdirs_(void (*cleanup) (void), const char *dir)
 		SAFE_MKDIR(cleanup, dir, 0744);
 
 	SAFE_STAT(cleanup, dir, &s);
-	if (!S_ISDIR(s.st_mode))
+	if (!S_ISDIR(s.st_mode)) {
 		tst_brkm(TBROK, cleanup, "%s is not directory", dir);
+		return 0;
+	}
 
 	/* for current kernel, subdir limit is not availiable for all fs */
 	fs_type = tst_fs_type(cleanup, dir);
@@ -144,6 +150,7 @@ int tst_fs_fill_subdirs_(void (*cleanup) (void), const char *dir)
 				tst_brkm(TBROK, cleanup, "%s link counts have"
 					 "%d, should be %d", dir,
 					 (int)s.st_nlink, i + 2);
+				return 0;
 			} else {
 				tst_resm(TINFO, "the maximum subdirectories in "
 				 "%s is hit: %d", dir, (int)s.st_nlink);
@@ -158,6 +165,7 @@ int tst_fs_fill_subdirs_(void (*cleanup) (void), const char *dir)
 			tst_brkm(TBROK, cleanup, "mkdir(%s, 0755) failed "
 				 "unexpectedly: %s", dirname,
 				 strerror(errno));
+			return 0;
 		}
 
 	}
diff --git a/lib/tst_fs_type.c b/lib/tst_fs_type.c
index c6c61ec..5fbf0f4 100644
--- a/lib/tst_fs_type.c
+++ b/lib/tst_fs_type.c
@@ -37,6 +37,7 @@ long tst_fs_type_(void (*cleanup)(void), const char *path)
 	if (statfs(path, &sbuf)) {
 		tst_brkm(TBROK | TERRNO, cleanup,
 		         "tst_fs_type: Failed to statfs(%s)", path);
+		return 0;
 	}
 
 	return sbuf.f_type;
diff --git a/lib/tst_kernel.c b/lib/tst_kernel.c
index 7a53002..71303fc 100644
--- a/lib/tst_kernel.c
+++ b/lib/tst_kernel.c
@@ -24,8 +24,10 @@ int tst_kernel_bits(void)
 	struct utsname buf;
 	int kernel_bits;
 
-	if (uname(&buf))
+	if (uname(&buf)) {
 		tst_brkm(TBROK | TERRNO, NULL, "uname()");
+		return -1;
+	}
 
 	kernel_bits = strstr(buf.machine, "64") ? 64 : 32;
 
diff --git a/lib/tst_mkfs.c b/lib/tst_mkfs.c
index 7810654..f2e40ec 100644
--- a/lib/tst_mkfs.c
+++ b/lib/tst_mkfs.c
@@ -33,11 +33,13 @@ void tst_mkfs_(const char *file, const int lineno, void (cleanup_fn)(void),
 	if (!dev) {
 		tst_brkm(TBROK, cleanup_fn,
 			 "%s:%d: No device specified", file, lineno);
+		return;
 	}
 
 	if (!fs_type) {
 		tst_brkm(TBROK, cleanup_fn,
 			 "%s:%d: No fs_type specified", file, lineno);
+		return;
 	}
 
 	snprintf(mkfs, sizeof(mkfs), "mkfs.%s", fs_type);
@@ -50,6 +52,7 @@ void tst_mkfs_(const char *file, const int lineno, void (cleanup_fn)(void),
 				tst_brkm(TBROK, cleanup_fn,
 				         "%s:%d: Too much mkfs options",
 					 file, lineno);
+				return;
 			}
 
 			if (i)
@@ -66,6 +69,7 @@ void tst_mkfs_(const char *file, const int lineno, void (cleanup_fn)(void),
 		if (pos + 1 > OPTS_MAX) {
 			tst_brkm(TBROK, cleanup_fn,
 			         "%s:%d: Too much mkfs options", file, lineno);
+			return;
 		}
 	}
 
diff --git a/lib/tst_module.c b/lib/tst_module.c
index fd4f019..ed39952 100644
--- a/lib/tst_module.c
+++ b/lib/tst_module.c
@@ -47,6 +47,7 @@ void tst_module_exists(void (cleanup_fn)(void),
 			tst_brkm(TBROK | TERRNO, cleanup_fn,
 				"asprintf failed at %s:%d",
 				__FILE__, __LINE__);
+			return;
 		}
 		err = access(buf, F_OK);
 	}
@@ -58,6 +59,7 @@ void tst_module_exists(void (cleanup_fn)(void),
 			tst_brkm(TBROK | TERRNO, cleanup_fn,
 				"asprintf failed at %s:%d",
 				__FILE__, __LINE__);
+			return;
 		}
 		err = access(buf, F_OK);
 	}
@@ -66,6 +68,7 @@ void tst_module_exists(void (cleanup_fn)(void),
 		free(buf);
 		tst_brkm(TCONF, cleanup_fn, "Failed to find module '%s'",
 			mod_name);
+		return;
 	}
 
 	if (mod_path != NULL)
diff --git a/lib/tst_net.c b/lib/tst_net.c
index f55e107..f842e94 100644
--- a/lib/tst_net.c
+++ b/lib/tst_net.c
@@ -54,20 +54,29 @@ unsigned short tst_get_unused_port(void (cleanup_fn)(void),
 	default:
 		tst_brkm(TBROK, cleanup_fn,
 			"tst_get_unused_port unknown family");
+		return -1;
 	}
 
 	sock = socket(addr->sa_family, type, 0);
-	if (sock < 0)
+	if (sock < 0) {
 		tst_brkm(TBROK | TERRNO, cleanup_fn, "socket failed");
+		return -1;
+	}
 
-	if (bind(sock, addr, slen) < 0)
+	if (bind(sock, addr, slen) < 0) {
 		tst_brkm(TBROK | TERRNO, cleanup_fn, "bind failed");
+		return -1;
+	}
 
-	if (getsockname(sock, addr, &slen) == -1)
+	if (getsockname(sock, addr, &slen) == -1) {
 		tst_brkm(TBROK | TERRNO, cleanup_fn, "getsockname failed");
+		return -1;
+	}
 
-	if (close(sock) == -1)
+	if (close(sock) == -1) {
 		tst_brkm(TBROK | TERRNO, cleanup_fn, "close failed");
+		return -1;
+	}
 
 	switch (family) {
 	case AF_INET:
diff --git a/lib/tst_path_has_mnt_flags.c b/lib/tst_path_has_mnt_flags.c
index ba66105..ea910ea 100644
--- a/lib/tst_path_has_mnt_flags.c
+++ b/lib/tst_path_has_mnt_flags.c
@@ -44,12 +44,14 @@ int tst_path_has_mnt_flags(void (cleanup_fn)(void),
 	if (access(path, F_OK) == -1) {
 		tst_brkm(TBROK | TERRNO, cleanup_fn,
 			"tst_path_has_mnt_flags: path %s doesn't exist", path);
+		return -1;
 	}
 
 	f = setmntent("/proc/mounts", "r");
 	if (f == NULL) {
 		tst_brkm(TBROK | TERRNO, cleanup_fn,
 			"tst_path_has_mnt_flags: failed to open /proc/mounts");
+		return -1;
 	}
 
 	while ((mnt = getmntent(f))) {
diff --git a/lib/tst_resource.c b/lib/tst_resource.c
index e283f86..78450df 100644
--- a/lib/tst_resource.c
+++ b/lib/tst_resource.c
@@ -54,9 +54,11 @@ static void tst_dataroot_init(void)
 	} else {
 		startdir = tst_get_startwd();
 		if (startdir[0] == 0) {
-			if (getcwd(curdir, PATH_MAX) == NULL)
+			if (getcwd(curdir, PATH_MAX) == NULL) {
 				tst_brkm(TBROK | TERRNO, NULL,
 					"tst_dataroot getcwd");
+				return;
+			}
 			startdir = curdir;
 		}
 		ret = snprintf(dataroot, PATH_MAX, "%s/datafiles", startdir);
@@ -103,6 +105,7 @@ void tst_resource_copy(const char *file, const int lineno,
 		tst_brkm(TBROK, cleanup_fn,
 		         "Temporary directory doesn't exist at %s:%d",
 		         file, lineno);
+		return;
 	}
 
 	if (dest == NULL)
diff --git a/lib/tst_run_cmd.c b/lib/tst_run_cmd.c
index 6fae573..8e4bf6b 100644
--- a/lib/tst_run_cmd.c
+++ b/lib/tst_run_cmd.c
@@ -42,6 +42,7 @@ int tst_run_cmd_fds_(void (cleanup_fn)(void),
 	if (argv == NULL || argv[0] == NULL) {
 		tst_brkm(TBROK, cleanup_fn,
 			"argument list is empty at %s:%d", __FILE__, __LINE__);
+		return -1;
 	}
 
 	/*
@@ -58,6 +59,7 @@ int tst_run_cmd_fds_(void (cleanup_fn)(void),
 	if (pid == -1) {
 		tst_brkm(TBROK | TERRNO, cleanup_fn, "vfork failed at %s:%d",
 			__FILE__, __LINE__);
+		return -1;
 	}
 	if (!pid) {
 		/* redirecting stdout and stderr if needed */
@@ -82,6 +84,7 @@ int tst_run_cmd_fds_(void (cleanup_fn)(void),
 	if (waitpid(pid, &ret, 0) != pid) {
 		tst_brkm(TBROK | TERRNO, cleanup_fn, "waitpid failed at %s:%d",
 			__FILE__, __LINE__);
+		return -1;
 	}
 
 	signal(SIGCHLD, old_handler);
@@ -89,14 +92,17 @@ int tst_run_cmd_fds_(void (cleanup_fn)(void),
 	if (!WIFEXITED(ret)) {
 		tst_brkm(TBROK, cleanup_fn, "failed to exec cmd '%s' at %s:%d",
 			argv[0], __FILE__, __LINE__);
+		return -1;
 	}
 
 	rc = WEXITSTATUS(ret);
 
-	if ((!pass_exit_val) && rc)
+	if ((!pass_exit_val) && rc) {
 		tst_brkm(TBROK, cleanup_fn,
 			 "'%s' exited with a non-zero code %d@%s:%d",
 			 argv[0], rc, __FILE__, __LINE__);
+		return -1;
+	}
 
 	return rc;
 }
diff --git a/lib/tst_timer.c b/lib/tst_timer.c
index 5f0cfc4..bd3f277 100644
--- a/lib/tst_timer.c
+++ b/lib/tst_timer.c
@@ -61,6 +61,7 @@ void tst_timer_check(clockid_t clk_id)
 			tst_brkm(TCONF, NULL,
 			         "Clock id %s(%u) not supported by kernel",
 				 clock_name(clk_id), clk_id);
+			return;
 		}
 
 		tst_brkm(TBROK | TERRNO, NULL, "clock_gettime() failed");
diff --git a/lib/tst_tmpdir.c b/lib/tst_tmpdir.c
index 405d377..824cade 100644
--- a/lib/tst_tmpdir.c
+++ b/lib/tst_tmpdir.c
@@ -105,9 +105,11 @@ int tst_tmpdir_created(void)
 
 char *tst_get_tmpdir(void)
 {
-	/* Smack the user for calling things out of order. */
-	if (TESTDIR == NULL)
+	if (TESTDIR == NULL) {
 		tst_brkm(TBROK, NULL, "you must call tst_tmpdir() first");
+		return NULL;
+	}
+
 	return strdup(TESTDIR);
 }
 
@@ -248,6 +250,7 @@ void tst_tmpdir(void)
 		if (c != env_tmpdir) {
 			tst_brkm(TBROK, NULL, "You must specify an absolute "
 				 "pathname for environment variable TMPDIR");
+			return;
 		}
 		snprintf(template, PATH_MAX, "%s/%.3sXXXXXX", env_tmpdir, TCID);
 	} else {
@@ -255,19 +258,29 @@ void tst_tmpdir(void)
 	}
 
 	/* Make the temporary directory in one shot using mkdtemp. */
-	if (mkdtemp(template) == NULL)
+	if (mkdtemp(template) == NULL) {
 		tst_brkm(TBROK | TERRNO, NULL,
 			 "%s: mkdtemp(%s) failed", __func__, template);
-	if ((TESTDIR = strdup(template)) == NULL)
+		return;
+	}
+
+	if ((TESTDIR = strdup(template)) == NULL) {
 		tst_brkm(TBROK | TERRNO, NULL,
 			 "%s: strdup(%s) failed", __func__, template);
+		return;
+	}
 
-	if (chown(TESTDIR, -1, getgid()) == -1)
+	if (chown(TESTDIR, -1, getgid()) == -1) {
 		tst_brkm(TBROK | TERRNO, NULL,
 			 "chown(%s, -1, %d) failed", TESTDIR, getgid());
-	if (chmod(TESTDIR, DIR_MODE) == -1)
+		return;
+	}
+
+	if (chmod(TESTDIR, DIR_MODE) == -1) {
 		tst_brkm(TBROK | TERRNO, NULL,
 			 "chmod(%s, %#o) failed", TESTDIR, DIR_MODE);
+		return;
+	}
 
 	if (getcwd(test_start_work_dir, sizeof(test_start_work_dir)) == NULL) {
 		tst_resm(TINFO, "Failed to record test working dir");
diff --git a/lib/tst_virt.c b/lib/tst_virt.c
index d40ba39..e95cf5e 100644
--- a/lib/tst_virt.c
+++ b/lib/tst_virt.c
@@ -115,5 +115,7 @@ int tst_is_virt(int virt_type)
 	case VIRT_KVM:
 		return is_kvm();
 	}
+
 	tst_brkm(TBROK, NULL, "invalid virt_type flag: %d", virt_type);
+	return -1;
 }
-- 
2.10.2

[LTP] [PATCH 3/4] newlib: Allow SAFE_MACROS to be called from cleanup

[Cyril Hrubis]

Which is done by:

* Dropping attribute ((noreturn)) from all tst_brk_() definitions so
  that the function could actually return in case it's called from
  cleanup.

* Adding brk handler to tst_test.c so that we can temporarily replace
  the tst_brk_() function with function that maps TBROK to TWARN and
  calls tst_vres_().

+ updated test-writing-guidelines

+ testcase

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 doc/test-writing-guidelines.txt | 11 ++++------
 include/tst_test.h              |  3 +--
 lib/ltp_priv.h                  |  2 +-
 lib/newlib_tests/.gitignore     |  1 +
 lib/newlib_tests/test14.c       | 45 +++++++++++++++++++++++++++++++++++++++++
 lib/tst_test.c                  | 23 +++++++++++++++++++--
 6 files changed, 73 insertions(+), 12 deletions(-)
 create mode 100644 lib/newlib_tests/test14.c

diff --git a/doc/test-writing-guidelines.txt b/doc/test-writing-guidelines.txt
index 41cee58..cec4bdc 100644
--- a/doc/test-writing-guidelines.txt
+++ b/doc/test-writing-guidelines.txt
@@ -341,9 +341,6 @@ cleanup() callback.
    initialized. (Some of the steps may not depend on others and everything
    will work if there were swapped but let's keep it in order.)
 
-3. Avoid using SAFE_MACROS() in cleanup if you want the cleanup to carry on
-   when a cleanup step has failed.
-
 The first rule may seem complicated at first however, on the contrary, it's
 quite easy. All you have to do is to keep track of what was already
 initialized. For example file descriptors needs to be closed only if they were
@@ -381,11 +378,11 @@ requires extra flag to be set after device was successfully mounted.
 -------------------------------------------------------------------------------
 static void cleanup(void)
 {
-	if (fd1 > 0 && close(fd1))
-		tst_res(TWARN | TERRNO, "close(fd1)");
+	if (fd1 > 0)
+		SAFE_CLOSE(fd1);
 
-	if (fd0 > 0 && close(fd0))
-		tst_res(TWARN | TERRNO, "close(fd0)");
+	if (fd0 > 0)
+		SAFE_CLOSE(fd0);
 
 	if (mount_flag && tst_umouont(MNTPOINT))
 		tst_res(TBROK | TERRNO, "umount(%s)", MNTPOINT);
diff --git a/include/tst_test.h b/include/tst_test.h
index d8cc806..1ed39db 100644
--- a/include/tst_test.h
+++ b/include/tst_test.h
@@ -62,8 +62,7 @@ void tst_resm_hexd_(const char *file, const int lineno, int ttype,
  */
 void tst_brk_(const char *file, const int lineno, int ttype,
               const char *fmt, ...)
-              __attribute__ ((format (printf, 4, 5)))
-              __attribute__ ((noreturn));
+              __attribute__ ((format (printf, 4, 5)));
 
 #define tst_brk(ttype, arg_fmt, ...) \
 	tst_brk_(__FILE__, __LINE__, (ttype), (arg_fmt), ##__VA_ARGS__)
diff --git a/lib/ltp_priv.h b/lib/ltp_priv.h
index de45e4a..e678a28 100644
--- a/lib/ltp_priv.h
+++ b/lib/ltp_priv.h
@@ -59,7 +59,7 @@ void tst_vbrk_(const char *file, const int lineno, int ttype,
                const char *fmt, va_list va) __attribute__((noreturn));
 
 void tst_brk_(const char *file, const int lineno, int ttype,
-              const char *msg, ...) __attribute__((noreturn));
+              const char *msg, ...);
 
 void tst_vres_(const char *file, const int lineno, int ttype,
                const char *fmt, va_list va);
diff --git a/lib/newlib_tests/.gitignore b/lib/newlib_tests/.gitignore
index bc2409c..8e12007 100644
--- a/lib/newlib_tests/.gitignore
+++ b/lib/newlib_tests/.gitignore
@@ -11,6 +11,7 @@ test10
 test11
 test12
 test13
+test14
 tst_device
 tst_safe_fileops
 tst_res_hexd
diff --git a/lib/newlib_tests/test14.c b/lib/newlib_tests/test14.c
new file mode 100644
index 0000000..4d94978
--- /dev/null
+++ b/lib/newlib_tests/test14.c
@@ -0,0 +1,45 @@
+/*
+ * Copyright (c) 2016 Cyril Hrubis <chrubis@suse.cz>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write the Free Software Foundation,
+ * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+#include "tst_test.h"
+#include "tst_safe_stdio.h"
+#include "tst_safe_net.h"
+
+static void cleanup(void)
+{
+	int i;
+
+	tst_brk(TBROK, "TBROK in cleanup");
+	SAFE_OPEN("foo", O_RDWR);
+	SAFE_FILE_SCANF("foo", "%i", &i);
+	SAFE_TOUCH("doo/foo", 0777, NULL);
+	SAFE_FOPEN("foo", "r");
+	SAFE_SOCKET(AF_UNIX, SOCK_STREAM, -1);
+	tst_res(TINFO, "Test still here");
+}
+
+static void do_test(void)
+{
+	tst_res(TPASS, "Passed");
+}
+
+static struct tst_test test = {
+	.tid = "test14",
+	.test_all = do_test,
+	.cleanup = cleanup,
+};
diff --git a/lib/tst_test.c b/lib/tst_test.c
index 8e24a94..f0f2c24 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -239,12 +239,31 @@ void tst_vres_(const char *file, const int lineno, int ttype,
 }
 
 void tst_vbrk_(const char *file, const int lineno, int ttype,
-               const char *fmt, va_list va) __attribute__((noreturn));
+               const char *fmt, va_list va);
+
+static void (*tst_brk_handler)(const char *file, const int lineno, int ttype,
+			       const char *fmt, va_list va) = tst_vbrk_;
+
+static void tst_cvres(const char *file, const int lineno, int ttype,
+		      const char *fmt, va_list va)
+{
+	if (TTYPE_RESULT(ttype) == TBROK) {
+		ttype &= ~TTYPE_MASK;
+		ttype |= TWARN;
+	}
+
+	print_result(file, lineno, ttype, fmt, va);
+	update_results(TTYPE_RESULT(ttype));
+}
 
 static void do_test_cleanup(void)
 {
+	tst_brk_handler = tst_cvres;
+
 	if (tst_test->cleanup)
 		tst_test->cleanup();
+
+	tst_brk_handler = tst_vbrk_;
 }
 
 void tst_vbrk_(const char *file, const int lineno, int ttype,
@@ -277,7 +296,7 @@ void tst_brk_(const char *file, const int lineno, int ttype,
 	va_list va;
 
 	va_start(va, fmt);
-	tst_vbrk_(file, lineno, ttype, fmt, va);
+	tst_brk_handler(file, lineno, ttype, fmt, va);
 	va_end(va);
 }
 
-- 
2.10.2

[LTP] [PATCH 4/4] syscalls: Make use of SAFE_MACROS in cleanup

[Cyril Hrubis]

Convert newlib tests to use SAFE_MACROS() in cleanup where possible.

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 testcases/kernel/syscalls/epoll_ctl/epoll_ctl01.c   | 12 ++++++------
 testcases/kernel/syscalls/epoll_ctl/epoll_ctl02.c   | 12 ++++++------
 testcases/kernel/syscalls/fcntl/fcntl02.c           |  4 ++--
 testcases/kernel/syscalls/fcntl/fcntl03.c           |  4 ++--
 testcases/kernel/syscalls/fcntl/fcntl04.c           |  4 ++--
 testcases/kernel/syscalls/flistxattr/flistxattr01.c |  4 ++--
 testcases/kernel/syscalls/flistxattr/flistxattr02.c |  4 ++--
 testcases/kernel/syscalls/flistxattr/flistxattr03.c |  6 ++----
 testcases/kernel/syscalls/ipc/msgget/msgget01.c     |  6 ++----
 testcases/kernel/syscalls/ipc/msgget/msgget02.c     |  6 ++----
 testcases/kernel/syscalls/ipc/msgget/msgget03.c     |  8 +++-----
 testcases/kernel/syscalls/ipc/msgsnd/msgsnd01.c     |  6 ++----
 testcases/kernel/syscalls/ipc/msgsnd/msgsnd02.c     |  6 ++----
 testcases/kernel/syscalls/ipc/msgsnd/msgsnd05.c     |  6 ++----
 testcases/kernel/syscalls/ipc/msgsnd/msgsnd06.c     |  6 ++----
 testcases/kernel/syscalls/kcmp/kcmp01.c             |  4 ++--
 testcases/kernel/syscalls/kcmp/kcmp02.c             |  8 ++++----
 testcases/kernel/syscalls/madvise/madvise01.c       |  7 +++----
 testcases/kernel/syscalls/madvise/madvise02.c       |  4 ++--
 testcases/kernel/syscalls/madvise/madvise06.c       | 11 +++--------
 testcases/kernel/syscalls/madvise/madvise08.c       | 19 ++++++++-----------
 testcases/kernel/syscalls/ppoll/ppoll01.c           |  2 +-
 testcases/kernel/syscalls/preadv/preadv01.c         |  4 ++--
 testcases/kernel/syscalls/preadv/preadv02.c         | 20 ++++++++++----------
 testcases/kernel/syscalls/pwritev/pwritev01.c       |  4 ++--
 testcases/kernel/syscalls/pwritev/pwritev02.c       | 16 ++++++++--------
 testcases/kernel/syscalls/recvmsg/recvmsg02.c       |  8 ++++----
 testcases/kernel/syscalls/sendto/sendto02.c         |  4 ++--
 testcases/kernel/syscalls/socket/socket02.c         |  4 ++--
 testcases/kernel/syscalls/socketpair/socketpair02.c |  8 ++++----
 testcases/kernel/syscalls/umount/umount02.c         |  4 ++--
 31 files changed, 98 insertions(+), 123 deletions(-)

diff --git a/testcases/kernel/syscalls/epoll_ctl/epoll_ctl01.c b/testcases/kernel/syscalls/epoll_ctl/epoll_ctl01.c
index 2ecadf7..5833c38 100644
--- a/testcases/kernel/syscalls/epoll_ctl/epoll_ctl01.c
+++ b/testcases/kernel/syscalls/epoll_ctl/epoll_ctl01.c
@@ -59,14 +59,14 @@ static void setup(void)
 
 static void cleanup(void)
 {
-	if (epfd > 0 && close(epfd))
-		tst_res(TWARN | TERRNO, "failed to close epoll instance");
+	if (epfd > 0)
+		SAFE_CLOSE(epfd);
 
-	if (fd[0] > 0 && close(fd[0]))
-		tst_res(TWARN | TERRNO, "failed to close pipe");
+	if (fd[0] > 0)
+		SAFE_CLOSE(fd[0]);
 
-	if (fd[1] > 0 && close(fd[1]))
-		tst_res(TWARN | TERRNO, "failed to close pipe");
+	if (fd[1] > 0)
+		SAFE_CLOSE(fd[1]);
 }
 
 static int has_event(struct epoll_event *epvs, int len,
diff --git a/testcases/kernel/syscalls/epoll_ctl/epoll_ctl02.c b/testcases/kernel/syscalls/epoll_ctl/epoll_ctl02.c
index ae2465c..360ae47 100644
--- a/testcases/kernel/syscalls/epoll_ctl/epoll_ctl02.c
+++ b/testcases/kernel/syscalls/epoll_ctl/epoll_ctl02.c
@@ -87,14 +87,14 @@ static void setup(void)
 
 static void cleanup(void)
 {
-	if (epfd > 0 && close(epfd))
-		tst_res(TWARN | TERRNO, "failed to close epoll instance");
+	if (epfd)
+		SAFE_CLOSE(epfd);
 
-	if (fd[0] > 0 && close(fd[0]))
-		tst_res(TWARN | TERRNO, "failed to close pipe");
+	if (fd[0])
+		SAFE_CLOSE(fd[0]);
 
-	if (fd[1] > 0 && close(fd[1]))
-		tst_res(TWARN | TERRNO, "failed to close pipe");
+	if (fd[1])
+		SAFE_CLOSE(fd[1]);
 }
 
 static void verify_epoll_ctl(unsigned int n)
diff --git a/testcases/kernel/syscalls/fcntl/fcntl02.c b/testcases/kernel/syscalls/fcntl/fcntl02.c
index f297808..cffa921 100644
--- a/testcases/kernel/syscalls/fcntl/fcntl02.c
+++ b/testcases/kernel/syscalls/fcntl/fcntl02.c
@@ -80,8 +80,8 @@ static void setup(void)
 
 static void cleanup(void)
 {
-	if (fd > 0 && close(fd))
-		tst_res(TWARN | TERRNO, "close(fd) failed");
+	if (fd > 0)
+		SAFE_CLOSE(fd);
 }
 
 static struct tst_test test = {
diff --git a/testcases/kernel/syscalls/fcntl/fcntl03.c b/testcases/kernel/syscalls/fcntl/fcntl03.c
index 7471267..cf2f8cf 100644
--- a/testcases/kernel/syscalls/fcntl/fcntl03.c
+++ b/testcases/kernel/syscalls/fcntl/fcntl03.c
@@ -69,8 +69,8 @@ static void setup(void)
 
 static void cleanup(void)
 {
-	if (fd > 0 && close(fd))
-		tst_res(TWARN | TERRNO, "close(fd) failed");
+	if (fd > 0)
+		SAFE_CLOSE(fd);
 }
 
 static struct tst_test test = {
diff --git a/testcases/kernel/syscalls/fcntl/fcntl04.c b/testcases/kernel/syscalls/fcntl/fcntl04.c
index f92f8a0..cddc6c3 100644
--- a/testcases/kernel/syscalls/fcntl/fcntl04.c
+++ b/testcases/kernel/syscalls/fcntl/fcntl04.c
@@ -75,8 +75,8 @@ static void setup(void)
 
 static void cleanup(void)
 {
-	if (fd > 0 && close(fd))
-		tst_res(TWARN | TERRNO, "close(fd) failed");
+	if (fd > 0)
+		SAFE_CLOSE(fd);
 }
 
 static struct tst_test test = {
diff --git a/testcases/kernel/syscalls/flistxattr/flistxattr01.c b/testcases/kernel/syscalls/flistxattr/flistxattr01.c
index 563b410..5fa4e7f 100644
--- a/testcases/kernel/syscalls/flistxattr/flistxattr01.c
+++ b/testcases/kernel/syscalls/flistxattr/flistxattr01.c
@@ -85,8 +85,8 @@ static void setup(void)
 
 static void cleanup(void)
 {
-	if (fd > 0 && close(fd))
-		tst_res(TWARN | TERRNO, "failed to close file");
+	if (fd > 0)
+		SAFE_CLOSE(fd);
 }
 
 static struct tst_test test = {
diff --git a/testcases/kernel/syscalls/flistxattr/flistxattr02.c b/testcases/kernel/syscalls/flistxattr/flistxattr02.c
index ee2e43b..876d53a 100644
--- a/testcases/kernel/syscalls/flistxattr/flistxattr02.c
+++ b/testcases/kernel/syscalls/flistxattr/flistxattr02.c
@@ -87,8 +87,8 @@ static void setup(void)
 
 static void cleanup(void)
 {
-	if (fd1 > 0 && close(fd1))
-		tst_res(TWARN | TERRNO, "failed to close file");
+	if (fd1 > 0)
+		SAFE_CLOSE(fd1);
 }
 
 static struct tst_test test = {
diff --git a/testcases/kernel/syscalls/flistxattr/flistxattr03.c b/testcases/kernel/syscalls/flistxattr/flistxattr03.c
index b9c6b73..0fcf0d8 100644
--- a/testcases/kernel/syscalls/flistxattr/flistxattr03.c
+++ b/testcases/kernel/syscalls/flistxattr/flistxattr03.c
@@ -78,10 +78,8 @@ static void setup(void)
 
 static void cleanup(void)
 {
-	if (fd[1] > 0 && close(fd[1]))
-		tst_res(TWARN | TERRNO, "failed to close file");
-	if (fd[0] > 0 && close(fd[0]))
-		tst_res(TWARN | TERRNO, "failed to close file");
+	SAFE_CLOSE(fd[1]);
+	SAFE_CLOSE(fd[0]);
 }
 
 static struct tst_test test = {
diff --git a/testcases/kernel/syscalls/ipc/msgget/msgget01.c b/testcases/kernel/syscalls/ipc/msgget/msgget01.c
index 1e9bbeb..8e058c5 100644
--- a/testcases/kernel/syscalls/ipc/msgget/msgget01.c
+++ b/testcases/kernel/syscalls/ipc/msgget/msgget01.c
@@ -66,10 +66,8 @@ static void setup(void)
 
 static void cleanup(void)
 {
-	if (queue_id != -1 && msgctl(queue_id, IPC_RMID, NULL)) {
-		tst_res(TWARN | TERRNO, "failed to delete message queue %i",
-			queue_id);
-	}
+	if (queue_id != -1)
+		SAFE_MSGCTL(queue_id, IPC_RMID, NULL);
 }
 
 static struct tst_test test = {
diff --git a/testcases/kernel/syscalls/ipc/msgget/msgget02.c b/testcases/kernel/syscalls/ipc/msgget/msgget02.c
index 1886201..9cf1314 100644
--- a/testcases/kernel/syscalls/ipc/msgget/msgget02.c
+++ b/testcases/kernel/syscalls/ipc/msgget/msgget02.c
@@ -104,10 +104,8 @@ static void setup(void)
 
 static void cleanup(void)
 {
-	if (queue_id != -1 && msgctl(queue_id, IPC_RMID, NULL)) {
-		tst_res(TWARN | TERRNO, "failed to delete message queue %i",
-			queue_id);
-	}
+	if (queue_id != -1)
+		SAFE_MSGCTL(queue_id, IPC_RMID, NULL);
 }
 
 static struct tst_test test = {
diff --git a/testcases/kernel/syscalls/ipc/msgget/msgget03.c b/testcases/kernel/syscalls/ipc/msgget/msgget03.c
index 0b7c66a..97911f9 100644
--- a/testcases/kernel/syscalls/ipc/msgget/msgget03.c
+++ b/testcases/kernel/syscalls/ipc/msgget/msgget03.c
@@ -29,6 +29,7 @@
 #include <stdlib.h>
 
 #include "tst_test.h"
+#include "tst_safe_sysv_ipc.h"
 #include "libnewipc.h"
 
 static int maxmsgs;
@@ -79,11 +80,8 @@ static void cleanup(void)
 		return;
 
 	for (num = 0; num < maxmsgs; num++) {
-		if (queues[num] != -1 && msgctl(queues[num], IPC_RMID, NULL)) {
-			tst_res(TWARN | TERRNO,
-				"failed to delete message queue %i",
-				queues[num]);
-		}
+		if (queues[num] != -1)
+			SAFE_MSGCTL(queues[num], IPC_RMID, NULL);
 	}
 
 	free(queues);
diff --git a/testcases/kernel/syscalls/ipc/msgsnd/msgsnd01.c b/testcases/kernel/syscalls/ipc/msgsnd/msgsnd01.c
index 7101bdb..d605fca 100644
--- a/testcases/kernel/syscalls/ipc/msgsnd/msgsnd01.c
+++ b/testcases/kernel/syscalls/ipc/msgsnd/msgsnd01.c
@@ -65,10 +65,8 @@ static void setup(void)
 
 static void cleanup(void)
 {
-	if (queue_id != -1 && msgctl(queue_id, IPC_RMID, NULL)) {
-		tst_res(TWARN | TERRNO, "failed to delete message queue %i",
-			queue_id);
-	}
+	if (queue_id != -1)
+		SAFE_MSGCTL(queue_id, IPC_RMID, NULL);
 }
 
 static struct tst_test test = {
diff --git a/testcases/kernel/syscalls/ipc/msgsnd/msgsnd02.c b/testcases/kernel/syscalls/ipc/msgsnd/msgsnd02.c
index 5bdc6c8..5632079 100644
--- a/testcases/kernel/syscalls/ipc/msgsnd/msgsnd02.c
+++ b/testcases/kernel/syscalls/ipc/msgsnd/msgsnd02.c
@@ -117,10 +117,8 @@ static void setup(void)
 
 static void cleanup(void)
 {
-	if (queue_id != -1 && msgctl(queue_id, IPC_RMID, NULL)) {
-		tst_res(TWARN | TERRNO, "failed to delete message queue %i",
-			queue_id);
-	}
+	if (queue_id != -1)
+		SAFE_MSGCTL(queue_id, IPC_RMID, NULL);
 }
 
 static struct tst_test test = {
diff --git a/testcases/kernel/syscalls/ipc/msgsnd/msgsnd05.c b/testcases/kernel/syscalls/ipc/msgsnd/msgsnd05.c
index 3d13f3e..032c870 100644
--- a/testcases/kernel/syscalls/ipc/msgsnd/msgsnd05.c
+++ b/testcases/kernel/syscalls/ipc/msgsnd/msgsnd05.c
@@ -109,10 +109,8 @@ static void setup(void)
 
 static void cleanup(void)
 {
-	if (queue_id != -1 && msgctl(queue_id, IPC_RMID, NULL)) {
-		tst_res(TWARN | TERRNO, "failed to delete message queue %i",
-			queue_id);
-	}
+	if (queue_id != -1)
+		SAFE_MSGCTL(queue_id, IPC_RMID, NULL);
 }
 
 static struct tst_test test = {
diff --git a/testcases/kernel/syscalls/ipc/msgsnd/msgsnd06.c b/testcases/kernel/syscalls/ipc/msgsnd/msgsnd06.c
index 787cf97..ce7f046 100644
--- a/testcases/kernel/syscalls/ipc/msgsnd/msgsnd06.c
+++ b/testcases/kernel/syscalls/ipc/msgsnd/msgsnd06.c
@@ -83,10 +83,8 @@ static void setup(void)
 
 static void cleanup(void)
 {
-	if (queue_id != -1 && msgctl(queue_id, IPC_RMID, NULL)) {
-		tst_res(TWARN | TERRNO, "failed to delete message queue %i",
-			queue_id);
-	}
+	if (queue_id != -1)
+		SAFE_MSGCTL(queue_id, IPC_RMID, NULL);
 }
 
 static struct tst_test test = {
diff --git a/testcases/kernel/syscalls/kcmp/kcmp01.c b/testcases/kernel/syscalls/kcmp/kcmp01.c
index 6e977cb..0aa4da6 100644
--- a/testcases/kernel/syscalls/kcmp/kcmp01.c
+++ b/testcases/kernel/syscalls/kcmp/kcmp01.c
@@ -61,8 +61,8 @@ static void setup(void)
 
 static void cleanup(void)
 {
-	if (fd1 > 0 && close(fd1) < 0)
-		tst_res(TWARN | TERRNO, "close fd1 failed");
+	if (fd1 > 0)
+		SAFE_CLOSE(fd1);
 }
 
 static void do_child(const struct test_case *test)
diff --git a/testcases/kernel/syscalls/kcmp/kcmp02.c b/testcases/kernel/syscalls/kcmp/kcmp02.c
index 2962823..ad0b09d 100644
--- a/testcases/kernel/syscalls/kcmp/kcmp02.c
+++ b/testcases/kernel/syscalls/kcmp/kcmp02.c
@@ -73,11 +73,11 @@ static void setup(void)
 
 static void cleanup(void)
 {
-	if (fd1 > 0 && close(fd1) < 0)
-		tst_res(TWARN | TERRNO, "close fd1 failed");
+	if (fd1 > 0)
+		SAFE_CLOSE(fd1);
 
-	if (fd2 > 0 && close(fd2) < 0)
-		tst_res(TWARN | TERRNO, "close fd2 failed");
+	if (fd2 > 0)
+		SAFE_CLOSE(fd2);
 }
 
 static void verify_kcmp(unsigned int n)
diff --git a/testcases/kernel/syscalls/madvise/madvise01.c b/testcases/kernel/syscalls/madvise/madvise01.c
index 22fb2e6..4ec3ef2 100644
--- a/testcases/kernel/syscalls/madvise/madvise01.c
+++ b/testcases/kernel/syscalls/madvise/madvise01.c
@@ -94,10 +94,9 @@ static void setup(void)
 
 static void cleanup(void)
 {
-	munmap(sfile, st.st_size);
-	munmap(amem, st.st_size);
-	umount(TMP_DIR);
-	rmdir(TMP_DIR);
+	SAFE_MUNMAP(sfile, st.st_size);
+	SAFE_MUNMAP(amem, st.st_size);
+	SAFE_UMOUNT(TMP_DIR);
 }
 
 static void verify_madvise(unsigned int i)
diff --git a/testcases/kernel/syscalls/madvise/madvise02.c b/testcases/kernel/syscalls/madvise/madvise02.c
index 33b1736..592536d 100644
--- a/testcases/kernel/syscalls/madvise/madvise02.c
+++ b/testcases/kernel/syscalls/madvise/madvise02.c
@@ -175,8 +175,8 @@ static void advice_test(unsigned int i)
 static void cleanup(void)
 {
 	free(ptr_addr);
-	munmap(file1, st.st_size);
-	munmap(file2, st.st_size - pagesize);
+	SAFE_MUNMAP(file1, st.st_size);
+	SAFE_MUNMAP(file2, st.st_size - pagesize);
 }
 
 static struct tst_test test = {
diff --git a/testcases/kernel/syscalls/madvise/madvise06.c b/testcases/kernel/syscalls/madvise/madvise06.c
index e040b6f..7e75f14 100644
--- a/testcases/kernel/syscalls/madvise/madvise06.c
+++ b/testcases/kernel/syscalls/madvise/madvise06.c
@@ -93,14 +93,9 @@ static void setup(void)
 
 static void cleanup(void)
 {
-	FILE *f = fopen(MNT_NAME"/tasks", "w");
-
-	if (f) {
-		fprintf(f, "%d\n", getpid());
-		fclose(f);
-	}
-	rmdir(MNT_NAME"/"GROUP_NAME);
-	umount(MNT_NAME);
+	SAFE_FILE_PRINTF(MNT_NAME"/tasks", "%d\n", getpid());
+	SAFE_RMDIR(MNT_NAME"/"GROUP_NAME);
+	SAFE_UMOUNT(MNT_NAME);
 }
 
 static void dirty_pages(char *ptr, long size)
diff --git a/testcases/kernel/syscalls/madvise/madvise08.c b/testcases/kernel/syscalls/madvise/madvise08.c
index 9541a42..ebda3f2 100644
--- a/testcases/kernel/syscalls/madvise/madvise08.c
+++ b/testcases/kernel/syscalls/madvise/madvise08.c
@@ -108,19 +108,16 @@ static void setup(void)
 
 static void cleanup(void)
 {
-	if (cpattern) {
-		if (FILE_PRINTF(CORE_PATTERN, "%s", cpattern))
-			tst_res(TWARN,
-				"COULD NOT RESTORE " CORE_PATTERN "! It used to be '%s'",
-				cpattern);
-		free(cpattern);
-	}
+	if (cpattern)
+		SAFE_FILE_PRINTF(CORE_PATTERN, "%s", cpattern);
+
+	free(cpattern);
 
-	if (fmem && munmap(fmem, FMEMSIZE))
-		tst_res(TWARN | TERRNO, "munmap(fmem)");
+	if (fmem)
+		SAFE_MUNMAP(fmem, FMEMSIZE);
 
-	if (dfd > 0 && close(dfd))
-		tst_res(TWARN | TERRNO, "close(dfd)");
+	if (dfd > 0)
+		SAFE_CLOSE(dfd);
 }
 
 static int find_sequence(int pid)
diff --git a/testcases/kernel/syscalls/ppoll/ppoll01.c b/testcases/kernel/syscalls/ppoll/ppoll01.c
index 8d6d7e7..2cb336b 100644
--- a/testcases/kernel/syscalls/ppoll/ppoll01.c
+++ b/testcases/kernel/syscalls/ppoll/ppoll01.c
@@ -203,7 +203,7 @@ static void setup(void)
 static void cleanup(void)
 {
 	if (fd1 != -1)
-		close(fd1);
+		SAFE_CLOSE(fd1);
 }
 
 static void do_test(unsigned int i)
diff --git a/testcases/kernel/syscalls/preadv/preadv01.c b/testcases/kernel/syscalls/preadv/preadv01.c
index a51bd44..c2ba09e 100644
--- a/testcases/kernel/syscalls/preadv/preadv01.c
+++ b/testcases/kernel/syscalls/preadv/preadv01.c
@@ -108,8 +108,8 @@ void setup(void)
 
 void cleanup(void)
 {
-	if (fd > 0 && close(fd))
-		tst_res(TWARN | TERRNO, "Failed to close file");
+	if (fd > 0)
+		SAFE_CLOSE(fd);
 }
 
 static struct tst_test test = {
diff --git a/testcases/kernel/syscalls/preadv/preadv02.c b/testcases/kernel/syscalls/preadv/preadv02.c
index d1863a0..26ebfa3 100644
--- a/testcases/kernel/syscalls/preadv/preadv02.c
+++ b/testcases/kernel/syscalls/preadv/preadv02.c
@@ -113,20 +113,20 @@ static void setup(void)
 
 static void cleanup(void)
 {
-	if (fd1 > 0 && close(fd1))
-		tst_res(TWARN | TERRNO, "failed to close file");
+	if (fd1 > 0)
+		SAFE_CLOSE(fd1);
 
-	if (fd2 > 0 && close(fd2))
-		tst_res(TWARN | TERRNO, "failed to close file");
+	if (fd2 > 0)
+		SAFE_CLOSE(fd2);
 
-	if (fd4 > 0 && close(fd4))
-		tst_res(TWARN | TERRNO, "failed to close file");
+	if (fd4 > 0)
+		SAFE_CLOSE(fd4);
 
-	if (fd5[0] > 0 && close(fd5[0]))
-		tst_res(TWARN | TERRNO, "failed to close file");
+	if (fd5[0] > 0)
+		SAFE_CLOSE(fd5[0]);
 
-	if (fd5[1] > 0 && close(fd5[1]))
-		tst_res(TWARN | TERRNO, "failed to close file");
+	if (fd5[1] > 0)
+		SAFE_CLOSE(fd5[1]);
 }
 
 static struct tst_test test = {
diff --git a/testcases/kernel/syscalls/pwritev/pwritev01.c b/testcases/kernel/syscalls/pwritev/pwritev01.c
index 75baf25..88a1756 100644
--- a/testcases/kernel/syscalls/pwritev/pwritev01.c
+++ b/testcases/kernel/syscalls/pwritev/pwritev01.c
@@ -104,8 +104,8 @@ static void setup(void)
 
 static void cleanup(void)
 {
-	if (fd > 0 && close(fd))
-		tst_res(TWARN | TERRNO, "failed to close file");
+	if (fd > 0)
+		SAFE_CLOSE(fd);
 }
 
 static struct tst_test test = {
diff --git a/testcases/kernel/syscalls/pwritev/pwritev02.c b/testcases/kernel/syscalls/pwritev/pwritev02.c
index 63444eb..ec9de20 100644
--- a/testcases/kernel/syscalls/pwritev/pwritev02.c
+++ b/testcases/kernel/syscalls/pwritev/pwritev02.c
@@ -107,17 +107,17 @@ static void setup(void)
 
 static void cleanup(void)
 {
-	if (fd1 > 0 && close(fd1))
-		tst_res(TWARN | TERRNO, "failed to close file");
+	if (fd1 > 0)
+		SAFE_CLOSE(fd1);
 
-	if (fd2 > 0 && close(fd2))
-		tst_res(TWARN | TERRNO, "failed to close file");
+	if (fd2 > 0)
+		SAFE_CLOSE(fd2);
 
-	if (fd4[0] > 0 && close(fd4[0]))
-		tst_res(TWARN | TERRNO, "failed to close file");
+	if (fd4[0] > 0)
+		SAFE_CLOSE(fd4[0]);
 
-	if (fd4[1] > 0 && close(fd4[1]))
-		tst_res(TWARN | TERRNO, "failed to close file");
+	if (fd4[1] > 0)
+		SAFE_CLOSE(fd4[1]);
 }
 
 static struct tst_test test = {
diff --git a/testcases/kernel/syscalls/recvmsg/recvmsg02.c b/testcases/kernel/syscalls/recvmsg/recvmsg02.c
index fd21092..9ed520b 100644
--- a/testcases/kernel/syscalls/recvmsg/recvmsg02.c
+++ b/testcases/kernel/syscalls/recvmsg/recvmsg02.c
@@ -98,11 +98,11 @@ static void verify_recvmsg(void)
 
 static void cleanup(void)
 {
-	if (sdw > 0 && close(sdw))
-		tst_res(TWARN | TERRNO, "close(sdw) failed");
+	if (sdw > 0)
+		SAFE_CLOSE(sdw);
 
-	if (sdr > 0 && close(sdr))
-		tst_res(TWARN | TERRNO, "close(sdr) failed");
+	if (sdr > 0)
+		SAFE_CLOSE(sdr);
 }
 
 static struct tst_test test = {
diff --git a/testcases/kernel/syscalls/sendto/sendto02.c b/testcases/kernel/syscalls/sendto/sendto02.c
index 2d5887d..b7b59fc 100644
--- a/testcases/kernel/syscalls/sendto/sendto02.c
+++ b/testcases/kernel/syscalls/sendto/sendto02.c
@@ -61,8 +61,8 @@ static void setup(void)
 
 static void cleanup(void)
 {
-	if (sockfd > 0 && close(sockfd))
-		tst_res(TWARN | TERRNO, "failed to close file");
+	if (sockfd > 0)
+		SAFE_CLOSE(sockfd);
 }
 
 static void verify_sendto(void)
diff --git a/testcases/kernel/syscalls/socket/socket02.c b/testcases/kernel/syscalls/socket/socket02.c
index 38a7cb5..ecc5b01 100644
--- a/testcases/kernel/syscalls/socket/socket02.c
+++ b/testcases/kernel/syscalls/socket/socket02.c
@@ -74,8 +74,8 @@ static void verify_socket(unsigned int n)
 
 static void cleanup(void)
 {
-	if (fd > 0 && close(fd))
-		tst_res(TWARN | TERRNO, "failed to close file");
+	if (fd > 0)
+		SAFE_CLOSE(fd);
 }
 
 static struct tst_test test = {
diff --git a/testcases/kernel/syscalls/socketpair/socketpair02.c b/testcases/kernel/syscalls/socketpair/socketpair02.c
index 783aea6..6507e31 100644
--- a/testcases/kernel/syscalls/socketpair/socketpair02.c
+++ b/testcases/kernel/syscalls/socketpair/socketpair02.c
@@ -84,11 +84,11 @@ ret:
 
 static void cleanup(void)
 {
-	if (fds[0] > 0 && close(fds[0]))
-		tst_res(TWARN | TERRNO, "failed to close file");
+	if (fds[0] > 0)
+		SAFE_CLOSE(fds[0]);
 
-	if (fds[1] > 0 && close(fds[1]))
-		tst_res(TWARN | TERRNO, "failed to close file");
+	if (fds[1] > 0)
+		SAFE_CLOSE(fds[1]);
 }
 
 static struct tst_test test = {
diff --git a/testcases/kernel/syscalls/umount/umount02.c b/testcases/kernel/syscalls/umount/umount02.c
index 41f13e4..ecfad1b 100644
--- a/testcases/kernel/syscalls/umount/umount02.c
+++ b/testcases/kernel/syscalls/umount/umount02.c
@@ -86,8 +86,8 @@ static void setup(void)
 
 static void cleanup(void)
 {
-	if (fd > 0 && close(fd))
-		tst_res(TWARN | TERRNO, "Failed to close file");
+	if (fd > 0)
+		SAFE_CLOSE(fd);
 
 	if (mount_flag)
 		tst_umount(MNTPOINT);
-- 
2.10.2

[LTP] [PATCH 2/4] lib: Redirect to tst_brk_() early

[Jan Stancek]

----- Original Message -----
> From: "Cyril Hrubis" <chrubis@suse.cz>
> To: ltp@lists.linux.it
> Sent: Tuesday, 14 February, 2017 1:26:37 PM
> Subject: [LTP] [PATCH 2/4] lib: Redirect to tst_brk_() early
> 
> This is first patch of a patchset that would allow us to use SAFE_MACROS() in
> newlib testcases. After the patch we redirect to tst_brk_() in case of newlib
> tests directly in the test library code.
> 
> This is needed since the tst_brkm_() from the old library is marked as
> attribute ((noreturn)) and because of that the return address is not
> saved on stack and hence we cannot return from the function. Removing
> the atrribute is not an option either since that generates ~1000
> "control reaches end of non-void function" warnings.
> 
> This commit redefines tst_brkm in test.h for the test library code so that we
> branch depending on if we are running oldlib/newlib testcase and call
> corresponding tst_brk_() or tst_brkm_() function directly instead of
> branching
> in the tst_brkm_() code.
> 
> We also had to add a few returns to various places in the test library so
> that
> we exit corresponding function in a case that tst_brkm() actually returned.
> 
> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>

1/4 and 2/4 look good to me, and match exactly what we discussed.
Reply to 3/4 and 4/4 will follow shortly.

Build on RHEL5.6 6.0 and 7.3 passes, but I noticed couple additional warnings:

tst_test.c: In function ‘print_result’:
tst_test.c:175: warning: ‘res’ may be used uninitialized in this function

test08.c: In function ‘worker’:
test08.c:54: warning: control reaches end of non-void function

netstress.c: In function ‘server_fn’:
netstress.c:539: warning: control reaches end of non-void function

Regards,
Jan

[LTP] [PATCH 2/4] lib: Redirect to tst_brk_() early

[Cyril Hrubis]

Hi!
> > We also had to add a few returns to various places in the test library so
> > that
> > we exit corresponding function in a case that tst_brkm() actually returned.
> > 
> > Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> 
> 1/4 and 2/4 look good to me, and match exactly what we discussed.
> Reply to 3/4 and 4/4 will follow shortly.
> 
> Build on RHEL5.6 6.0 and 7.3 passes, but I noticed couple additional warnings:
> 
> tst_test.c: In function ???print_result???:
> tst_test.c:175: warning: ???res??? may be used uninitialized in this function

What about adding an abort() after the tst_brk() in the switch?

> test08.c: In function ???worker???:
> test08.c:54: warning: control reaches end of non-void function

That should be fixed easily with return NULL; at the end of the
function.

> netstress.c: In function ???server_fn???:
> netstress.c:539: warning: control reaches end of non-void function

Dummy return NULL should fix this one as well.



These fixes should be part of 3/4 that removes the attribute noreturn
from the tst_brk_().

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 3/4] newlib: Allow SAFE_MACROS to be called from cleanup

[Jan Stancek]

----- Original Message -----
> diff --git a/doc/test-writing-guidelines.txt
> b/doc/test-writing-guidelines.txt
> index 41cee58..cec4bdc 100644
> --- a/doc/test-writing-guidelines.txt
> +++ b/doc/test-writing-guidelines.txt
> @@ -341,9 +341,6 @@ cleanup() callback.
>     initialized. (Some of the steps may not depend on others and everything
>     will work if there were swapped but let's keep it in order.)
>  
> -3. Avoid using SAFE_MACROS() in cleanup if you want the cleanup to carry on
> -   when a cleanup step has failed.
> -
>  The first rule may seem complicated at first however, on the contrary, it's
>  quite easy. All you have to do is to keep track of what was already
>  initialized. For example file descriptors needs to be closed only if they
>  were
> @@ -381,11 +378,11 @@ requires extra flag to be set after device was
> successfully mounted.

Code is straight-forward, but maybe we should add a sentence or two
to docs to clarify what happens when you call SAFE_MACRO in cleanup:
1) cleanup continues to run and
2) TBROK errors are reported as TWARN

Regards,
Jan

[LTP] [PATCH 3/4] newlib: Allow SAFE_MACROS to be called from cleanup

[Cyril Hrubis]

Hi!
> Code is straight-forward, but maybe we should add a sentence or two
> to docs to clarify what happens when you call SAFE_MACRO in cleanup:
> 1) cleanup continues to run and
> 2) TBROK errors are reported as TWARN

Sure, will add that.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 2/4] lib: 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: Tuesday, 14 February, 2017 2:22:44 PM
> Subject: Re: [LTP] [PATCH 2/4] lib: Redirect to tst_brk_() early
> 
> Hi!
> > > We also had to add a few returns to various places in the test library so
> > > that
> > > we exit corresponding function in a case that tst_brkm() actually
> > > returned.
> > > 
> > > Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> > 
> > 1/4 and 2/4 look good to me, and match exactly what we discussed.
> > Reply to 3/4 and 4/4 will follow shortly.
> > 
> > Build on RHEL5.6 6.0 and 7.3 passes, but I noticed couple additional
> > warnings:
> > 
> > tst_test.c: In function ???print_result???:
> > tst_test.c:175: warning: ???res??? may be used uninitialized in this
> > function
> 
> What about adding an abort() after the tst_brk() in the switch?

Agreed, if this fails there's something very bad going on.

> 
> > test08.c: In function ???worker???:
> > test08.c:54: warning: control reaches end of non-void function
> 
> That should be fixed easily with return NULL; at the end of the
> function.
> 
> > netstress.c: In function ???server_fn???:
> > netstress.c:539: warning: control reaches end of non-void function
> 
> Dummy return NULL should fix this one as well.
> 
> 
> 
> These fixes should be part of 3/4 that removes the attribute noreturn
> from the tst_brk_().
> 
> --
> Cyril Hrubis
> chrubis@suse.cz
> 

[LTP] [PATCH 4/4] syscalls: Make use of SAFE_MACROS in cleanup

[Jan Stancek]

----- Original Message -----
> From: "Cyril Hrubis" <chrubis@suse.cz>
> To: ltp@lists.linux.it
> Sent: Tuesday, 14 February, 2017 1:26:39 PM
> Subject: [LTP] [PATCH 4/4] syscalls: Make use of SAFE_MACROS in cleanup
> 
> Convert newlib tests to use SAFE_MACROS() in cleanup where possible.
> 
> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>

Looks good to me.

I also ran all syscalls tests on 4.9 (with RHEL7.4 userspace).
There were 2 known failures: utimensat01 and madvise07

[LTP] [PATCH 3/4] newlib: Allow SAFE_MACROS to be called from cleanup

[Cyril Hrubis]

Hi!
OK to commit the patchset with fixed warnings and this addition to the docs?

 -------------------------------------------------------------------------------
 static void cleanup(void)
 {
-       if (fd1 > 0 && close(fd1))
-               tst_res(TWARN | TERRNO, "close(fd1)");
+       if (fd1 > 0)
+               SAFE_CLOSE(fd1);
 
-       if (fd0 > 0 && close(fd0))
-               tst_res(TWARN | TERRNO, "close(fd0)");
+       if (fd0 > 0)
+               SAFE_CLOSE(fd0);
 
        if (mount_flag && tst_umouont(MNTPOINT))
-               tst_res(TBROK | TERRNO, "umount(%s)", MNTPOINT);
+               tst_res(TWARN | TERRNO, "umount(%s)", MNTPOINT);
 }
 -------------------------------------------------------------------------------
 
+IMPORTANT: 'SAFE_MACROS()' used in cleanup *do not* exit the test. Failure
+           only produces a warning and the 'cleanup()' carries on. This is
+          intentional as we want to execute as much 'cleanup()' as possible.
+
+WARNING: Calling tst_brk() in test 'cleanup()' does not exit the test as well
+         and 'TBROK' is converted to 'TWARN'.
+


-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 3/4] newlib: Allow SAFE_MACROS to be called from cleanup

[Jan Stancek]

----- Original Message -----
> From: "Cyril Hrubis" <chrubis@suse.cz>
> To: "Jan Stancek" <jstancek@redhat.com>
> Cc: ltp@lists.linux.it
> Sent: Tuesday, 14 February, 2017 2:52:31 PM
> Subject: Re: [LTP] [PATCH 3/4] newlib: Allow SAFE_MACROS to be called from cleanup
> 
> Hi!
> OK to commit the patchset with fixed warnings and this addition to the docs?

ACK, thanks!

[LTP] [PATCH 3/4] newlib: Allow SAFE_MACROS to be called from cleanup

[Cyril Hrubis]

Hi!
> ACK, thanks!

Done, thanks for the review.

-- 
Cyril Hrubis
chrubis@suse.cz