[LTP] [PATCH v2 1/7] Unify error handling in lib/tst_safe_sysv_ipc.c

[LTP] [PATCH v2 1/7] Unify error handling in lib/tst_safe_sysv_ipc.c

[Martin Doucha]

- Properly format caller file:line locations
- Pedantically check for invalid syscall return values, don't ignore silently

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

Changes since v1:
- Added missing "else" keyword

 lib/tst_safe_sysv_ipc.c | 80 ++++++++++++++++++++++++++++-------------
 1 file changed, 56 insertions(+), 24 deletions(-)

diff --git a/lib/tst_safe_sysv_ipc.c b/lib/tst_safe_sysv_ipc.c
index 30b5f6ec7..155e03b5e 100644
--- a/lib/tst_safe_sysv_ipc.c
+++ b/lib/tst_safe_sysv_ipc.c
@@ -25,7 +25,7 @@ static int ret_check(int cmd, int ret)
 	case IPC_RMID:
 		return ret != 0;
 	default:
-		return ret == -1;
+		return ret < 0;
 	}
 }
 
@@ -34,9 +34,14 @@ int safe_msgget(const char *file, const int lineno, key_t key, int msgflg)
 	int rval;
 
 	rval = msgget(key, msgflg);
+
 	if (rval == -1) {
-		tst_brk(TBROK | TERRNO, "%s:%d: msgget(%i, %x) failed",
-			file, lineno, (int)key, msgflg);
+		tst_brk_(file, lineno, TBROK | TERRNO, "msgget(%i, %x) failed",
+			(int)key, msgflg);
+	} else if (rval < 0) {
+		tst_brk_(file, lineno, TBROK | TERRNO,
+			"Invalid msgget(%i, %x) return value %d", (int)key,
+			msgflg, rval);
 	}
 
 	return rval;
@@ -48,10 +53,15 @@ int safe_msgsnd(const char *file, const int lineno, int msqid, const void *msgp,
 	int rval;
 
 	rval = msgsnd(msqid, msgp, msgsz, msgflg);
+
 	if (rval == -1) {
-		tst_brk(TBROK | TERRNO,
-			"%s:%d: msgsnd(%i, %p, %zu, %x) failed",
-			file, lineno, msqid, msgp, msgsz, msgflg);
+		tst_brk_(file, lineno, TBROK | TERRNO,
+			"msgsnd(%i, %p, %zu, %x) failed", msqid, msgp, msgsz,
+			msgflg);
+	} else if (rval) {
+		tst_brk_(file, lineno, TBROK | TERRNO,
+			"Invalid msgsnd(%i, %p, %zu, %x) return value %d",
+			msqid, msgp, msgsz, msgflg, rval);
 	}
 
 	return rval;
@@ -63,10 +73,15 @@ ssize_t safe_msgrcv(const char *file, const int lineno, int msqid, void *msgp,
 	ssize_t rval;
 
 	rval = msgrcv(msqid, msgp, msgsz, msgtyp, msgflg);
+
 	if (rval == -1) {
-		tst_brk(TBROK | TERRNO,
-			"%s:%d: msgrcv(%i, %p, %zu, %li, %x) failed",
-			file, lineno, msqid, msgp, msgsz, msgtyp, msgflg);
+		tst_brk_(file, lineno, TBROK | TERRNO,
+			"msgrcv(%i, %p, %zu, %li, %x) failed",
+			msqid, msgp, msgsz, msgtyp, msgflg);
+	} else if (rval < 0) {
+		tst_brk_(file, lineno, TBROK | TERRNO,
+			"Invalid msgrcv(%i, %p, %zu, %li, %x) return value %ld",
+			msqid, msgp, msgsz, msgtyp, msgflg, rval);
 	}
 
 	return rval;
@@ -78,12 +93,15 @@ int safe_msgctl(const char *file, const int lineno, int msqid, int cmd,
 	int rval;
 
 	rval = msgctl(msqid, cmd, buf);
-	if (ret_check(cmd, rval)) {
-		tst_brk(TBROK | TERRNO,
-			"%s:%d: msgctl(%i, %i, %p) = %i failed",
-			file, lineno, msqid, cmd, buf, rval);
-	}
 
+	if (rval == -1) {
+		tst_brk_(file, lineno, TBROK | TERRNO,
+			"msgctl(%i, %i, %p) failed", msqid, cmd, buf);
+	} else if (ret_check(cmd, rval)) {
+		tst_brk_(file, lineno, TBROK | TERRNO,
+			"Invalid msgctl(%i, %i, %p) return value %d", msqid,
+			cmd, buf, rval);
+	}
 
 	return rval;
 }
@@ -94,9 +112,14 @@ int safe_shmget(const char *file, const int lineno, key_t key, size_t size,
 	int rval;
 
 	rval = shmget(key, size, shmflg);
+
 	if (rval == -1) {
-		tst_brk(TBROK | TERRNO, "%s:%d: shmget(%i, %zu, %x) failed",
-			file, lineno, (int)key, size, shmflg);
+		tst_brk_(file, lineno, TBROK | TERRNO,
+			"shmget(%i, %zu, %x) failed", (int)key, size, shmflg);
+	} else if (rval < 0) {
+		tst_brk_(file, lineno, TBROK | TERRNO,
+			"Invalid shmget(%i, %zu, %x) return value %d",
+			(int)key, size, shmflg, rval);
 	}
 
 	return rval;
@@ -108,9 +131,10 @@ void *safe_shmat(const char *file, const int lineno, int shmid,
 	void *rval;
 
 	rval = shmat(shmid, shmaddr, shmflg);
+
 	if (rval == (void *)-1) {
-		tst_brk(TBROK | TERRNO, "%s:%d: shmat(%i, %p, %x) failed",
-			file, lineno, shmid, shmaddr, shmflg);
+		tst_brk_(file, lineno, TBROK | TERRNO,
+			"shmat(%i, %p, %x) failed", shmid, shmaddr, shmflg);
 	}
 
 	return rval;
@@ -121,9 +145,13 @@ int safe_shmdt(const char *file, const int lineno, const void *shmaddr)
 	int rval;
 
 	rval = shmdt(shmaddr);
+
 	if (rval == -1) {
-		tst_brk(TBROK | TERRNO, "%s:%d: shmdt(%p) failed",
-			file, lineno, shmaddr);
+		tst_brk_(file, lineno, TBROK | TERRNO, "shmdt(%p) failed",
+			shmaddr);
+	} else if (rval) {
+		tst_brk_(file, lineno, TBROK | TERRNO,
+			"Invalid shmdt(%p) return value %d", shmaddr, rval);
 	}
 
 	return rval;
@@ -135,10 +163,14 @@ int safe_shmctl(const char *file, const int lineno, int shmid, int cmd,
 	int rval;
 
 	rval = shmctl(shmid, cmd, buf);
-	if (ret_check(cmd, rval)) {
-		tst_brk(TBROK | TERRNO,
-			"%s:%d: shmctl(%i, %i, %p) = %i failed",
-			file, lineno, shmid, cmd, buf, rval);
+
+	if (rval == -1) {
+		tst_brk_(file, lineno, TBROK | TERRNO,
+			"shmctl(%i, %i, %p) failed", shmid, cmd, buf);
+	} else if (ret_check(cmd, rval)) {
+		tst_brk_(file, lineno, TBROK | TERRNO,
+			"Invalid shmctl(%i, %i, %p) return value %d", shmid,
+			cmd, buf, rval);
 	}
 
 	return rval;
-- 
2.28.0

[LTP] [PATCH v2 2/7] Unify error handling in lib/tst_safe_timerfd.c

[Martin Doucha]

- Properly format caller file:line locations
- Pedantically check for invalid syscall return values

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

Changes since v1:
- Added missing "else" keyword

 lib/tst_safe_timerfd.c | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/lib/tst_safe_timerfd.c b/lib/tst_safe_timerfd.c
index ffe7b2ef7..d31f6e35e 100644
--- a/lib/tst_safe_timerfd.c
+++ b/lib/tst_safe_timerfd.c
@@ -17,9 +17,14 @@ int safe_timerfd_create(const char *file, const int lineno,
 	int fd;
 
 	fd = timerfd_create(clockid, flags);
-	if (fd < 0) {
-		tst_brk(TTYPE | TERRNO, "%s:%d timerfd_create(%s) failed",
-			file, lineno, tst_clock_name(clockid));
+
+	if (fd == -1) {
+		tst_brk_(file, lineno, TTYPE | TERRNO,
+			"timerfd_create(%s) failed", tst_clock_name(clockid));
+	} else if (fd < 0) {
+		tst_brk_(file, lineno, TBROK | TERRNO,
+			"Invalid timerfd_create(%s) return value %d",
+			tst_clock_name(clockid), fd);
 	}
 
 	return fd;
@@ -31,9 +36,13 @@ int safe_timerfd_gettime(const char *file, const int lineno,
 	int rval;
 
 	rval = timerfd_gettime(fd, curr_value);
-	if (rval != 0) {
-		tst_brk(TTYPE | TERRNO, "%s:%d timerfd_gettime() failed",
-			file, lineno);
+
+	if (rval == -1) {
+		tst_brk_(file, lineno, TTYPE | TERRNO,
+			"timerfd_gettime() failed");
+	} else if (rval) {
+		tst_brk_(file, lineno, TBROK | TERRNO,
+			"Invalid timerfd_gettime() return value %d", rval);
 	}
 
 	return rval;
@@ -47,9 +56,13 @@ int safe_timerfd_settime(const char *file, const int lineno,
 	int rval;
 
 	rval = timerfd_settime(fd, flags, new_value, old_value);
-	if (rval != 0) {
-		tst_brk(TTYPE | TERRNO, "%s:%d timerfd_settime() failed",
-			file, lineno);
+
+	if (rval == -1) {
+		tst_brk_(file, lineno, TTYPE | TERRNO,
+			"timerfd_settime() failed");
+	} else if (rval) {
+		tst_brk_(file, lineno, TBROK | TERRNO,
+			"Invalid timerfd_settime() return value %d", rval);
 	}
 
 	return rval;
-- 
2.28.0

[LTP] [PATCH v2 3/7] Unify error handling in lib/safe_file_ops.c

[Martin Doucha]

- Properly format caller file:line location
- Pedantically check invalid syscall return values
- Always return success/failure value so that all SAFE_*() functions can be
  called in test cleanup

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

Changes since v1:
- Dropped return type changes in safe_file_scanf() and safe_file_printf()
- Code style fixes

 include/safe_file_ops_fn.h |   4 +-
 lib/safe_file_ops.c        | 187 +++++++++++++++++++++----------------
 2 files changed, 106 insertions(+), 85 deletions(-)

diff --git a/include/safe_file_ops_fn.h b/include/safe_file_ops_fn.h
index 052fb1b9a..ed7d978dd 100644
--- a/include/safe_file_ops_fn.h
+++ b/include/safe_file_ops_fn.h
@@ -55,7 +55,7 @@ void safe_file_printf(const char *file, const int lineno,
 /*
  * Safe function to copy files, no more system("cp ...") please.
  */
-void safe_cp(const char *file, const int lineno,
+int safe_cp(const char *file, const int lineno,
              void (*cleanup_fn)(void),
 	     const char *src, const char *dst);
 
@@ -71,7 +71,7 @@ void safe_cp(const char *file, const int lineno,
  * times is a timespec[2] (as for utimensat(2)). If times is NULL then
  * the access/modification times of the file is set to the current time.
  */
-void safe_touch(const char *file, const int lineno,
+int safe_touch(const char *file, const int lineno,
 		void (*cleanup_fn)(void),
 		const char *pathname,
 		mode_t mode, const struct timespec times[2]);
diff --git a/lib/safe_file_ops.c b/lib/safe_file_ops.c
index e06d399fa..0ec2ff8fe 100644
--- a/lib/safe_file_ops.c
+++ b/lib/safe_file_ops.c
@@ -84,9 +84,8 @@ int file_scanf(const char *file, const int lineno,
 	f = fopen(path, "r");
 
 	if (f == NULL) {
-		tst_resm(TWARN,
-			"Failed to open FILE '%s' at %s:%d",
-			 path, file, lineno);
+		tst_resm_(file, lineno, TWARN, "Failed to open FILE '%s'",
+			path);
 		return 1;
 	}
 
@@ -97,23 +96,21 @@ int file_scanf(const char *file, const int lineno,
 	va_end(va);
 
 	if (ret == EOF) {
-		tst_resm(TWARN,
-			 "The FILE '%s' ended prematurely at %s:%d",
-			 path, file, lineno);
+		tst_resm_(file, lineno, TWARN,
+			"The FILE '%s' ended prematurely", path);
 		goto err;
 	}
 
 	if (ret != exp_convs) {
-		tst_resm(TWARN,
-			"Expected %i conversions got %i FILE '%s' at %s:%d",
-			 exp_convs, ret, path, file, lineno);
+		tst_resm_(file, lineno, TWARN,
+			"Expected %i conversions got %i FILE '%s'",
+			exp_convs, ret, path);
 		goto err;
 	}
 
 	if (fclose(f)) {
-		tst_resm(TWARN,
-			 "Failed to close FILE '%s' at %s:%d",
-			 path, file, lineno);
+		tst_resm_(file, lineno, TWARN, "Failed to close FILE '%s'",
+			path);
 		return 1;
 	}
 
@@ -121,10 +118,10 @@ int file_scanf(const char *file, const int lineno,
 
 err:
 	if (fclose(f)) {
-		tst_resm(TWARN,
-			 "Failed to close FILE '%s' at %s:%d",
-			 path, file, lineno);
+		tst_resm_(file, lineno, TWARN, "Failed to close FILE '%s'",
+			path);
 	}
+
 	return 1;
 }
 
@@ -139,9 +136,8 @@ void safe_file_scanf(const char *file, const int lineno,
 	f = fopen(path, "r");
 
 	if (f == NULL) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "Failed to open FILE '%s' for reading at %s:%d",
-			 path, file, lineno);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Failed to open FILE '%s' for reading", path);
 		return;
 	}
 
@@ -152,23 +148,21 @@ void safe_file_scanf(const char *file, const int lineno,
 	va_end(va);
 
 	if (ret == EOF) {
-		tst_brkm(TBROK, cleanup_fn,
-			 "The FILE '%s' ended prematurely at %s:%d",
-			 path, file, lineno);
+		tst_brkm_(file, lineno, TBROK, cleanup_fn,
+			"The FILE '%s' ended prematurely", path);
 		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);
+		tst_brkm_(file, lineno, TBROK, cleanup_fn,
+			"Expected %i conversions got %i FILE '%s'",
+			exp_convs, ret, path);
 		return;
 	}
 
 	if (fclose(f)) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "Failed to close FILE '%s' at %s:%d",
-			 path, file, lineno);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Failed to close FILE '%s'", path);
 		return;
 	}
 }
@@ -190,16 +184,14 @@ int file_lines_scanf(const char *file, const int lineno,
 	va_list ap;
 
 	if (!fmt) {
-		tst_brkm(TBROK, cleanup_fn, "pattern is NULL, %s:%d",
-			file, lineno);
+		tst_brkm_(file, lineno, TBROK, cleanup_fn, "pattern is NULL");
 		return 1;
 	}
 
 	fp = fopen(path, "r");
 	if (fp == NULL) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			"Failed to open FILE '%s' for reading at %s:%d",
-			path, file, lineno);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Failed to open FILE '%s' for reading", path);
 		return 1;
 	}
 
@@ -216,8 +208,9 @@ int file_lines_scanf(const char *file, const int lineno,
 	fclose(fp);
 
 	if (strict && ret != arg_count) {
-		tst_brkm(TBROK, cleanup_fn, "Expected %i conversions got %i"
-			" FILE '%s' at %s:%d", arg_count, ret, path, file, lineno);
+		tst_brkm_(file, lineno, TBROK, cleanup_fn,
+			"Expected %i conversions got %i FILE '%s'",
+			arg_count, ret, path);
 		return 1;
 	}
 
@@ -233,27 +226,24 @@ int file_printf(const char *file, const int lineno,
 	f = fopen(path, "w");
 
 	if (f == NULL) {
-		tst_resm(TWARN,
-			 "Failed to open FILE '%s' at %s:%d",
-			 path, file, lineno);
+		tst_resm_(file, lineno, TWARN, "Failed to open FILE '%s'",
+			path);
 		return 1;
 	}
 
 	va_start(va, fmt);
 
 	if (vfprintf(f, fmt, va) < 0) {
-		tst_resm(TWARN,
-			"Failed to print to FILE '%s' at %s:%d",
-			 path, file, lineno);
+		tst_resm_(file, lineno, TWARN, "Failed to print to FILE '%s'",
+			path);
 		goto err;
 	}
 
 	va_end(va);
 
 	if (fclose(f)) {
-		tst_resm(TWARN,
-			 "Failed to close FILE '%s' at %s:%d",
-			 path, file, lineno);
+		tst_resm_(file, lineno, TWARN, "Failed to close FILE '%s'",
+			path);
 		return 1;
 	}
 
@@ -261,10 +251,10 @@ int file_printf(const char *file, const int lineno,
 
 err:
 	if (fclose(f)) {
-		tst_resm(TWARN,
-			 "Failed to close FILE '%s' at %s:%d",
-			 path, file, lineno);
+		tst_resm_(file, lineno, TWARN, "Failed to close FILE '%s'",
+			path);
 	}
+
 	return 1;
 }
 
@@ -278,33 +268,30 @@ void safe_file_printf(const char *file, const int lineno,
 	f = fopen(path, "w");
 
 	if (f == NULL) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "Failed to open FILE '%s' for writing at %s:%d",
-			 path, file, lineno);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Failed to open FILE '%s' for writing", path);
 		return;
 	}
 
 	va_start(va, fmt);
 
 	if (vfprintf(f, fmt, va) < 0) {
-		tst_brkm(TBROK, cleanup_fn,
-			 "Failed to print to FILE '%s' at %s:%d",
-			 path, file, lineno);
+		tst_brkm_(file, lineno, TBROK, cleanup_fn,
+			"Failed to print to FILE '%s'", path);
 		return;
 	}
 
 	va_end(va);
 
 	if (fclose(f)) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "Failed to close FILE '%s' at %s:%d",
-			 path, file, lineno);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Failed to close FILE '%s'", path);
 		return;
 	}
 }
 
 //TODO: C implementation? better error condition reporting?
-void safe_cp(const char *file, const int lineno,
+int safe_cp(const char *file, const int lineno,
 	     void (*cleanup_fn) (void), const char *src, const char *dst)
 {
 	size_t len = strlen(src) + strlen(dst) + 16;
@@ -316,10 +303,12 @@ void safe_cp(const char *file, const int lineno,
 	ret = system(buf);
 
 	if (ret) {
-		tst_brkm(TBROK, cleanup_fn,
-			 "Failed to copy '%s' to '%s' at %s:%d",
-			 src, dst, file, lineno);
+		tst_brkm_(file, lineno, TBROK, cleanup_fn,
+			"Failed to copy '%s' to '%s'", src, dst);
+		return ret;
 	}
+
+	return 0;
 }
 
 #ifndef HAVE_UTIMENSAT
@@ -342,7 +331,7 @@ static void set_time(struct timeval *res, const struct timespec *src,
 
 #endif
 
-void safe_touch(const char *file, const int lineno,
+int safe_touch(const char *file, const int lineno,
 		void (*cleanup_fn)(void),
 		const char *pathname,
 		mode_t mode, const struct timespec times[2])
@@ -353,28 +342,41 @@ 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) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			"Failed to open file '%s' at %s:%d",
-			pathname, file, lineno);
-		return;
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Failed to open file '%s'", pathname);
+		return ret;
+	} else if (ret < 0) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Invalid open(%s) return value %d", pathname, ret);
+		return ret;
 	}
 
 	ret = close(ret);
+
 	if (ret == -1) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			"Failed to close file '%s' at %s:%d",
-			pathname, file, lineno);
-		return;
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Failed to close file '%s'", pathname);
+		return ret;
+	} else if (ret) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Invalid close('%s') return value %d", pathname, ret);
+		return ret;
 	}
 
 	if (mode != 0) {
 		ret = chmod(pathname, mode);
+
 		if (ret == -1) {
-			tst_brkm(TBROK | TERRNO, cleanup_fn,
-				"Failed to chmod file '%s' at %s:%d",
-				pathname, file, lineno);
-			return;
+			tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+				"Failed to chmod file '%s'", pathname);
+			return ret;
+		} else if (ret) {
+			tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+				"Invalid chmod('%s') return value %d",
+				pathname, ret);
+			return ret;
 		}
 	}
 
@@ -389,19 +391,28 @@ void safe_touch(const char *file, const int lineno,
 		struct timeval cotimes[2];
 
 		ret = stat(pathname, &sb);
+
 		if (ret == -1) {
-			tst_brkm(TBROK | TERRNO, cleanup_fn,
-				"Failed to stat file '%s' at %s:%d",
-				pathname, file, lineno);
-			return;
+			tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+				"Failed to stat file '%s'", pathname);
+			return ret;
+		} else if (ret) {
+			tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+				"Invalid stat('%s') return value %d",
+				pathname, ret);
+			return ret;
 		}
 
 		ret = gettimeofday(cotimes, NULL);
+
 		if (ret == -1) {
-			tst_brkm(TBROK | TERRNO, cleanup_fn,
-				"Failed to gettimeofday() at %s:%d",
-				file, lineno);
-			return;
+			tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+				"Failed to gettimeofday()");
+			return ret;
+		} else if (ret) {
+			tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+				"Invalid gettimeofday() return value %d", ret);
+			return ret;
 		}
 
 		cotimes[1] = cotimes[0];
@@ -415,8 +426,18 @@ void safe_touch(const char *file, const int lineno,
 	}
 #endif
 	if (ret == -1) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			"Failed to update the access/modification time on file"
-			" '%s'@%s:%d", pathname, file, lineno);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Failed to update the access/modification time on file '%s'",
+			pathname);
+	} else if (ret) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+#ifdef HAVE_UTIMENSAT
+			"Invalid utimensat('%s') return value %d",
+#else
+			"Invalid utimes('%s') return value %d",
+#endif
+			pathname, ret);
 	}
+
+	return ret;
 }
-- 
2.28.0

[LTP] [PATCH v2 4/7] Unify error handling in lib/safe_macros.c

[Martin Doucha]

- Properly format caller file:line location
- Pedantically check invalid syscall return values

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

Changes since v1:
- Added missing "else" keyword
- Code style fixes
- Pedantically check invalid syscall return value in safe_closedir()
- Fixed safe_*xattr() control flow in cleanup if xattr is not supported

 lib/safe_macros.c | 605 +++++++++++++++++++++++++++++-----------------
 1 file changed, 386 insertions(+), 219 deletions(-)

diff --git a/lib/safe_macros.c b/lib/safe_macros.c
index 4f48d7529..9285fae93 100644
--- a/lib/safe_macros.c
+++ b/lib/safe_macros.c
@@ -30,10 +30,10 @@ char *safe_basename(const char *file, const int lineno,
 	char *rval;
 
 	rval = basename(path);
+
 	if (rval == NULL) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "%s:%d: basename(%s) failed",
-			 file, lineno, path);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"basename(%s) failed", path);
 	}
 
 	return rval;
@@ -46,10 +46,13 @@ safe_chdir(const char *file, const int lineno, void (*cleanup_fn) (void),
 	int rval;
 
 	rval = chdir(path);
+
 	if (rval == -1) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "%s:%d: chdir(%s) failed",
-			 file, lineno, path);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"chdir(%s) failed", path);
+	} else if (rval) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Invalid chdir(%s) return value %d", path, rval);
 	}
 
 	return rval;
@@ -62,10 +65,13 @@ safe_close(const char *file, const int lineno, void (*cleanup_fn) (void),
 	int rval;
 
 	rval = close(fildes);
+
 	if (rval == -1) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "%s:%d: close(%d) failed",
-			 file, lineno, fildes);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"close(%d) failed", fildes);
+	} else if (rval) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Invalid close(%d) return value %d", fildes, rval);
 	}
 
 	return rval;
@@ -78,10 +84,14 @@ safe_creat(const char *file, const int lineno, void (*cleanup_fn) (void),
 	int rval;
 
 	rval = creat(pathname, mode);
+
 	if (rval == -1) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "%s:%d: creat(%s,0%o) failed",
-			 file, lineno, pathname, mode);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"creat(%s,%04o) failed", pathname, mode);
+	} else if (rval < 0) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Invalid creat(%s,%04o) return value %d", pathname,
+			mode, rval);
 	}
 
 	return rval;
@@ -93,10 +103,10 @@ char *safe_dirname(const char *file, const int lineno,
 	char *rval;
 
 	rval = dirname(path);
+
 	if (rval == NULL) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "%s:%d: dirname(%s) failed",
-			 file, lineno, path);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"dirname(%s) failed", path);
 	}
 
 	return rval;
@@ -108,10 +118,10 @@ char *safe_getcwd(const char *file, const int lineno, void (*cleanup_fn) (void),
 	char *rval;
 
 	rval = getcwd(buf, size);
+
 	if (rval == NULL) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "%s:%d: getcwd(%p,%zu) failed",
-			 file, lineno, buf, size);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"getcwd(%p,%zu) failed", buf, size);
 	}
 
 	return rval;
@@ -123,10 +133,10 @@ struct passwd *safe_getpwnam(const char *file, const int lineno,
 	struct passwd *rval;
 
 	rval = getpwnam(name);
+
 	if (rval == NULL) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "%s:%d: getpwnam(%s) failed",
-			 file, lineno, name);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"getpwnam(%s) failed", name);
 	}
 
 	return rval;
@@ -139,10 +149,14 @@ safe_getrusage(const char *file, const int lineno, void (*cleanup_fn) (void),
 	int rval;
 
 	rval = getrusage(who, usage);
+
 	if (rval == -1) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "%s:%d: getrusage(%d,%p) failed",
-			 file, lineno, who, usage);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"getrusage(%d,%p) failed", who, usage);
+	} else if (rval) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Invalid getrusage(%d,%p) return value %d", who,
+			usage, rval);
 	}
 
 	return rval;
@@ -154,10 +168,10 @@ void *safe_malloc(const char *file, const int lineno, void (*cleanup_fn) (void),
 	void *rval;
 
 	rval = malloc(size);
+
 	if (rval == NULL) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "%s:%d: malloc(%zu) failed",
-			 file, lineno, size);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"malloc(%zu) failed", size);
 	}
 
 	return rval;
@@ -169,10 +183,14 @@ int safe_mkdir(const char *file, const int lineno, void (*cleanup_fn) (void),
 	int rval;
 
 	rval = mkdir(pathname, mode);
+
 	if (rval == -1) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "%s:%d: mkdir(%s,0%o) failed",
-			 file, lineno, pathname, mode);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"mkdir(%s, %04o) failed", pathname, mode);
+	} else if (rval) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Invalid mkdir(%s, %04o) return value %d", pathname,
+			mode, rval);
 	}
 
 	return (rval);
@@ -184,10 +202,13 @@ int safe_rmdir(const char *file, const int lineno, void (*cleanup_fn) (void),
 	int rval;
 
 	rval = rmdir(pathname);
+
 	if (rval == -1) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "%s:%d: rmdir(%s) failed",
-			 file, lineno, pathname);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"rmdir(%s) failed", pathname);
+	} else if (rval) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Invalid rmdir(%s) return value %d", pathname, rval);
 	}
 
 	return (rval);
@@ -199,10 +220,14 @@ int safe_munmap(const char *file, const int lineno, void (*cleanup_fn) (void),
 	int rval;
 
 	rval = munmap(addr, length);
+
 	if (rval == -1) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "%s:%d: munmap(%p,%zu) failed",
-			 file, lineno, addr, length);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"munmap(%p,%zu) failed", addr, length);
+	} else if (rval) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Invalid munmap(%p,%zu) return value %d", addr,
+			length, rval);
 	}
 
 	return rval;
@@ -225,10 +250,14 @@ int safe_open(const char *file, const int lineno, void (*cleanup_fn) (void),
 	va_end(ap);
 
 	rval = open(pathname, oflags, mode);
+
 	if (rval == -1) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "%s:%d: open(%s,%d,0%o) failed",
-			 file, lineno, pathname, oflags, mode);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"open(%s,%d,%04o) failed", pathname, oflags, mode);
+	} else if (rval < 0) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Invalid open(%s,%d,%04o) return value %d", pathname,
+			oflags, mode, rval);
 	}
 
 	return rval;
@@ -240,10 +269,14 @@ int safe_pipe(const char *file, const int lineno, void (*cleanup_fn) (void),
 	int rval;
 
 	rval = pipe(fildes);
+
 	if (rval == -1) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "%s:%d: pipe({%d,%d}) failed",
-			 file, lineno, fildes[0], fildes[1]);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"pipe({%d,%d}) failed", fildes[0], fildes[1]);
+	} else if (rval) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Invalid pipe({%d,%d}) return value %d", fildes[0],
+			fildes[1], rval);
 	}
 
 	return rval;
@@ -255,10 +288,15 @@ ssize_t safe_read(const char *file, const int lineno, void (*cleanup_fn) (void),
 	ssize_t rval;
 
 	rval = read(fildes, buf, nbyte);
+
 	if (rval == -1 || (len_strict && (size_t)rval != nbyte)) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "%s:%d: read(%d,%p,%zu) failed, returned %zd",
-			 file, lineno, fildes, buf, nbyte, rval);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"read(%d,%p,%zu) failed, returned %zd", fildes, buf,
+			nbyte, rval);
+	} else if (rval < 0) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Invalid read(%d,%p,%zu) return value %zd", fildes,
+			buf, nbyte, rval);
 	}
 
 	return rval;
@@ -270,10 +308,14 @@ int safe_setegid(const char *file, const int lineno, void (*cleanup_fn) (void),
 	int rval;
 
 	rval = setegid(egid);
+
 	if (rval == -1) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "%s:%d: setegid(%u) failed",
-			 file, lineno, (unsigned) egid);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"setegid(%u) failed", (unsigned int)egid);
+	} else if (rval) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Invalid setegid(%u) return value %d",
+			(unsigned int)egid, rval);
 	}
 
 	return rval;
@@ -285,10 +327,14 @@ int safe_seteuid(const char *file, const int lineno, void (*cleanup_fn) (void),
 	int rval;
 
 	rval = seteuid(euid);
+
 	if (rval == -1) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "%s:%d: seteuid(%u) failed",
-			 file, lineno, (unsigned) euid);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"seteuid(%u) failed", (unsigned int)euid);
+	} else if (rval) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Invalid seteuid(%u) return value %d",
+			(unsigned int)euid, rval);
 	}
 
 	return rval;
@@ -300,10 +346,14 @@ int safe_setgid(const char *file, const int lineno, void (*cleanup_fn) (void),
 	int rval;
 
 	rval = setgid(gid);
+
 	if (rval == -1) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "%s:%d: setgid(%u) failed",
-			 file, lineno, (unsigned) gid);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"setgid(%u) failed", (unsigned int)gid);
+	} else if (rval) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Invalid setgid(%u) return value %d",
+			(unsigned int)gid, rval);
 	}
 
 	return rval;
@@ -315,10 +365,14 @@ int safe_setuid(const char *file, const int lineno, void (*cleanup_fn) (void),
 	int rval;
 
 	rval = setuid(uid);
+
 	if (rval == -1) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "%s:%d: setuid(%u) failed",
-			 file, lineno, (unsigned) uid);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"setuid(%u) failed", (unsigned int)uid);
+	} else if (rval) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Invalid setuid(%u) return value %d",
+			(unsigned int)uid, rval);
 	}
 
 	return rval;
@@ -330,10 +384,14 @@ int safe_getresuid(const char *file, const int lineno, void (*cleanup_fn)(void),
 	int rval;
 
 	rval = getresuid(ruid, euid, suid);
+
 	if (rval == -1) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "%s:%d: getresuid(%p, %p, %p) failed",
-			 file, lineno, ruid, euid, suid);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"getresuid(%p, %p, %p) failed", ruid, euid, suid);
+	} else if (rval) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Invalid getresuid(%p, %p, %p) return value %d", ruid,
+			euid, suid, rval);
 	}
 
 	return rval;
@@ -345,10 +403,14 @@ int safe_getresgid(const char *file, const int lineno, void (*cleanup_fn)(void),
 	int rval;
 
 	rval = getresgid(rgid, egid, sgid);
+
 	if (rval == -1) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "%s:%d: getresgid(%p, %p, %p) failed",
-			 file, lineno, rgid, egid, sgid);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"getresgid(%p, %p, %p) failed", rgid, egid, sgid);
+	} else if (rval) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Invalid getresgid(%p, %p, %p) return value %d", rgid,
+			egid, sgid, rval);
 	}
 
 	return rval;
@@ -360,10 +422,13 @@ int safe_unlink(const char *file, const int lineno, void (*cleanup_fn) (void),
 	int rval;
 
 	rval = unlink(pathname);
+
 	if (rval == -1) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "%s:%d: unlink(%s) failed",
-			 file, lineno, pathname);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"unlink(%s) failed", pathname);
+	} else if (rval) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Invalid unlink(%s) return value %d", pathname, rval);
 	}
 
 	return rval;
@@ -379,9 +444,12 @@ int safe_link(const char *file, const int lineno,
 	rval = link(oldpath, newpath);
 
 	if (rval == -1) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-		         "%s:%d: link(%s,%s) failed",
-			 file, lineno, oldpath, newpath);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+		        "link(%s,%s) failed", oldpath, newpath);
+	} else if (rval) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+		        "Invalid link(%s,%s) return value %d", oldpath,
+			newpath, rval);
 	}
 
 	return rval;
@@ -396,10 +464,13 @@ int safe_linkat(const char *file, const int lineno,
 	rval = linkat(olddirfd, oldpath, newdirfd, newpath, flags);
 
 	if (rval == -1) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "%s:%d: linkat(%d,%s,%d,%s,%d) failed",
-			 file, lineno, olddirfd, oldpath, newdirfd,
-			 newpath, flags);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"linkat(%d,%s,%d,%s,%d) failed", olddirfd, oldpath,
+			newdirfd, newpath, flags);
+	} else if (rval) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Invalid linkat(%d,%s,%d,%s,%d) return value %d",
+			olddirfd, oldpath, newdirfd, newpath, flags, rval);
 	}
 
 	return rval;
@@ -414,9 +485,12 @@ ssize_t safe_readlink(const char *file, const int lineno,
 	rval = readlink(path, buf, bufsize);
 
 	if (rval == -1) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "%s:%d: readlink(%s,%p,%zu) failed",
-			 file, lineno, path, buf, bufsize);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"readlink(%s,%p,%zu) failed", path, buf, bufsize);
+	} else if (rval < 0) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Invalid readlink(%s,%p,%zu) return value %zd", path,
+			buf, bufsize, rval);
 	} else {
 		/* readlink does not append a NUL byte to the buffer.
 		 * Add it now. */
@@ -438,9 +512,12 @@ int safe_symlink(const char *file, const int lineno,
 	rval = symlink(oldpath, newpath);
 
 	if (rval == -1) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-		         "%s:%d: symlink(%s,%s) failed",
-			 file, lineno, oldpath, newpath);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"symlink(%s,%s) failed", oldpath, newpath);
+	} else if (rval) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Invalid symlink(%s,%s) return value %d", oldpath,
+			newpath, rval);
 	}
 
 	return rval;
@@ -452,10 +529,14 @@ ssize_t safe_write(const char *file, const int lineno, void (cleanup_fn) (void),
 	ssize_t rval;
 
 	rval = write(fildes, buf, nbyte);
+
 	if (rval == -1 || (len_strict && (size_t)rval != nbyte)) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "%s:%d: write(%d,%p,%zu) failed",
-		         file, lineno, fildes, buf, rval);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"write(%d,%p,%zu) failed", fildes, buf, nbyte);
+	} else if (rval < 0) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Invalid write(%d,%p,%zu) return value %zd", fildes,
+			buf, nbyte, rval);
 	}
 
 	return rval;
@@ -472,21 +553,21 @@ long safe_strtol(const char *file, const int lineno,
 
 	if ((errno == ERANGE && (rval == LONG_MAX || rval == LONG_MIN))
 	    || (errno != 0 && rval == 0)) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "%s:%d: strtol(%s) failed", file, lineno, str);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"strtol(%s) failed", str);
 		return rval;
 	}
 
 	if (endptr == str || (*endptr != '\0' && *endptr != '\n')) {
-		tst_brkm(TBROK, cleanup_fn,
-			 "%s:%d: strtol(%s): Invalid value", file, lineno, str);
+		tst_brkm_(file, lineno, TBROK, cleanup_fn,
+			"strtol(%s): Invalid value", 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);
+		tst_brkm_(file, lineno, TBROK, cleanup_fn,
+			"strtol(%s): %ld is out of range %ld - %ld",
+			str, rval, min, max);
 		return 0;
 	}
 
@@ -505,21 +586,21 @@ unsigned long safe_strtoul(const char *file, const int lineno,
 
 	if ((errno == ERANGE && rval == ULONG_MAX)
 	    || (errno != 0 && rval == 0)) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "%s:%d: strtoul(%s) failed", file, lineno, str);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"strtoul(%s) failed", 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);
+		tst_brkm_(file, lineno, TBROK, cleanup_fn,
+			"strtoul(%s): %lu is out of range %lu - %lu",
+			str, rval, min, max);
 		return 0;
 	}
 
 	if (endptr == str || (*endptr != '\0' && *endptr != '\n')) {
-		tst_brkm(TBROK, cleanup_fn,
-			 "Invalid value: '%s' at %s:%d", str, file, lineno);
+		tst_brkm_(file, lineno, TBROK, cleanup_fn,
+			"Invalid value: '%s'", str);
 		return 0;
 	}
 
@@ -530,20 +611,18 @@ long safe_sysconf(const char *file, const int lineno,
 		  void (cleanup_fn) (void), int name)
 {
 	long rval;
-	errno = 0;
 
+	errno = 0;
 	rval = sysconf(name);
 
 	if (rval == -1) {
 		if (errno) {
-			tst_brkm(TBROK | TERRNO, cleanup_fn,
-				 "%s:%d: sysconf(%d) failed",
-				 file, lineno, name);
+			tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+				"sysconf(%d) failed", name);
 		} else {
-			tst_resm(TINFO, "%s:%d: sysconf(%d): "
-				 "queried option is not available"
-				 " or there is no definite limit",
-				 file, lineno, name);
+			tst_resm_(file, lineno, TINFO,
+				"sysconf(%d): queried option is not available  or there is no definite limit",
+				name);
 		}
 	}
 
@@ -558,9 +637,12 @@ int safe_chmod(const char *file, const int lineno,
 	rval = chmod(path, mode);
 
 	if (rval == -1) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "%s:%d: chmod(%s,0%o) failed",
-			 file, lineno, path, mode);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"chmod(%s,%04o) failed", path, mode);
+	} else if (rval) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Invalid chmod(%s,%04o) return value %d", path, mode,
+			rval);
 	}
 
 	return rval;
@@ -574,9 +656,12 @@ int safe_fchmod(const char *file, const int lineno,
 	rval = fchmod(fd, mode);
 
 	if (rval == -1) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "%s:%d: fchmod(%d,0%o) failed",
-			 file, lineno, fd, mode);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"fchmod(%d,%04o) failed", fd, mode);
+	} else if (rval) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Invalid fchmod(%d,%04o) return value %d", fd, mode,
+			rval);
 	}
 
 	return rval;
@@ -590,9 +675,12 @@ int safe_chown(const char *file, const int lineno, void (cleanup_fn)(void),
 	rval = chown(path, owner, group);
 
 	if (rval == -1) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			"%s:%d: chown(%s,%d,%d) failed",
-			file, lineno, path, owner, group);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"chown(%s,%d,%d) failed", path, owner, group);
+	} else if (rval) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Invalid chown(%s,%d,%d) return value %d", path,
+			owner, group, rval);
 	}
 
 	return rval;
@@ -606,9 +694,12 @@ int safe_fchown(const char *file, const int lineno, void (cleanup_fn)(void),
 	rval = fchown(fd, owner, group);
 
 	if (rval == -1) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-		         "%s:%d: fchown(%d,%d,%d) failed",
-			 file, lineno, fd, owner, group);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"fchown(%d,%d,%d) failed", fd, owner, group);
+	} else if (rval) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Invalid fchown(%d,%d,%d) return value %d", fd,
+			owner, group, rval);
 	}
 
 	return rval;
@@ -620,10 +711,13 @@ pid_t safe_wait(const char *file, const int lineno, void (cleanup_fn)(void),
 	pid_t rval;
 
 	rval = wait(status);
+
 	if (rval == -1) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "%s:%d: wait(%p) failed",
-			 file, lineno, status);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"wait(%p) failed", status);
+	} else if (rval < 0) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Invalid wait(%p) return value %d", status, rval);
 	}
 
 	return rval;
@@ -635,10 +729,14 @@ pid_t safe_waitpid(const char *file, const int lineno, void (cleanup_fn)(void),
 	pid_t rval;
 
 	rval = waitpid(pid, status, opts);
+
 	if (rval == -1) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "%s:%d: waitpid(%d,%p,%d) failed",
-			 file, lineno, pid, status, opts);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"waitpid(%d,%p,%d) failed", pid, status, opts);
+	} else if (rval < 0) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Invalid waitpid(%d,%p,%d) return value %d", pid,
+			status, opts, rval);
 	}
 
 	return rval;
@@ -650,9 +748,11 @@ void *safe_memalign(const char *file, const int lineno,
 	void *rval;
 
 	rval = memalign(alignment, size);
-	if (rval == NULL)
-		tst_brkm(TBROK | TERRNO, cleanup_fn, "memalign failed@%s:%d",
-			 file, lineno);
+
+	if (rval == NULL) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"memalign() failed");
+	}
 
 	return rval;
 }
@@ -665,9 +765,12 @@ int safe_kill(const char *file, const int lineno, void (cleanup_fn)(void),
 	rval = kill(pid, sig);
 
 	if (rval == -1) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "%s:%d: kill(%d,%s) failed",
-			 file, lineno, pid, tst_strsig(sig));
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"kill(%d,%s) failed", pid, tst_strsig(sig));
+	} else if (rval) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Invalid kill(%d,%s) return value %d", pid,
+			tst_strsig(sig), rval);
 	}
 
 	return rval;
@@ -681,9 +784,12 @@ int safe_mkfifo(const char *file, const int lineno,
 	rval = mkfifo(pathname, mode);
 
 	if (rval == -1) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-		         "%s:%d: mkfifo(%s, 0%o) failed",
-			 file, lineno, pathname, mode);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"mkfifo(%s, %04o) failed", pathname, mode);
+	} else if (rval) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Invalid mkfifo(%s, %04o) return value %d", pathname,
+			mode, rval);
 	}
 
 	return rval;
@@ -697,9 +803,12 @@ int safe_rename(const char *file, const int lineno, void (*cleanup_fn)(void),
 	rval = rename(oldpath, newpath);
 
 	if (rval == -1) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "%s:%d: rename(%s, %s) failed",
-			 file, lineno, oldpath, newpath);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"rename(%s, %s) failed", oldpath, newpath);
+	} else if (rval) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Invalid rename(%s, %s) return value %d", oldpath,
+			newpath, rval);
 	}
 
 	return rval;
@@ -730,7 +839,7 @@ int safe_mount(const char *file, const int lineno, void (*cleanup_fn)(void),
 	       const char *filesystemtype, unsigned long mountflags,
 	       const void *data)
 {
-	int rval;
+	int rval = -1;
 
 	/*
 	 * Don't try using the kernel's NTFS driver when mounting NTFS, since
@@ -752,25 +861,29 @@ int safe_mount(const char *file, const int lineno, void (*cleanup_fn)(void),
 	if (possibly_fuse(filesystemtype)) {
 		char buf[1024];
 
-		tst_resm(TINFO, "Trying FUSE...");
+		tst_resm_(file, lineno, TINFO, "Trying FUSE...");
 		snprintf(buf, sizeof(buf), "mount.%s '%s' '%s'",
-			 filesystemtype, source, target);
+			filesystemtype, source, target);
 
 		rval = tst_system(buf);
 		if (WIFEXITED(rval) && WEXITSTATUS(rval) == 0)
 			return 0;
 
-		tst_brkm(TBROK, cleanup_fn, "mount.%s failed with %i",
-			 filesystemtype, rval);
+		tst_brkm_(file, lineno, TBROK, cleanup_fn,
+			"mount.%s failed with %i", filesystemtype, rval);
 		return -1;
+	} else if (rval == -1) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"mount(%s, %s, %s, %lu, %p) failed", source, target,
+			filesystemtype, mountflags, data);
 	} else {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "%s:%d: mount(%s, %s, %s, %lu, %p) failed",
-			 file, lineno, source, target, filesystemtype,
-			 mountflags, data);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Invalid mount(%s, %s, %s, %lu, %p) return value %d",
+			source, target, filesystemtype, mountflags, data,
+			rval);
 	}
 
-	return -1;
+	return rval;
 }
 
 int safe_umount(const char *file, const int lineno, void (*cleanup_fn)(void),
@@ -781,9 +894,11 @@ int safe_umount(const char *file, const int lineno, void (*cleanup_fn)(void),
 	rval = tst_umount(target);
 
 	if (rval == -1) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "%s:%d: umount(%s) failed",
-			 file, lineno, target);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"umount(%s) failed", target);
+	} else if (rval) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Invalid umount(%s) return value %d", target, rval);
 	}
 
 	return rval;
@@ -797,8 +912,8 @@ DIR* safe_opendir(const char *file, const int lineno, void (cleanup_fn)(void),
 	rval = opendir(name);
 
 	if (!rval) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-		         "%s:%d: opendir(%s) failed", file, lineno, name);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"opendir(%s) failed", name);
 	}
 
 	return rval;
@@ -811,9 +926,12 @@ int safe_closedir(const char *file, const int lineno, void (cleanup_fn)(void),
 
 	rval = closedir(dirp);
 
-	if (rval) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-		         "%s:%d: closedir(%p) failed", file, lineno, dirp);
+	if (rval == -1) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"closedir(%p) failed", dirp);
+	} else if (rval) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Invalid closedir(%p) return value %d", dirp, rval);
 	}
 
 	return rval;
@@ -829,8 +947,8 @@ struct dirent *safe_readdir(const char *file, const int lineno, void (cleanup_fn
 	rval = readdir(dirp);
 
 	if (!rval && errno) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-		         "%s:%d: readdir(%p) failed", file, lineno, dirp);
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"readdir(%p) failed", dirp);
 	}
 
 	errno = err;
@@ -843,10 +961,14 @@ int safe_getpriority(const char *file, const int lineno, int which, id_t who)
 
 	errno = 0;
 	rval = getpriority(which, who);
-	if (errno) {
-		tst_brkm(TBROK | TERRNO, NULL,
-		         "%s:%d getpriority(%i, %i) failed",
-			 file, lineno, which, who);
+
+	if (rval == -1 && errno) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+			"getpriority(%i, %i) failed", which, who);
+	} else if (errno) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+			"getpriority(%i, %i) failed with return value %d",
+			which, who, rval);
 	}
 
 	errno = err;
@@ -862,14 +984,18 @@ ssize_t safe_getxattr(const char *file, const int lineno, const char *path,
 
 	if (rval == -1) {
 		if (errno == ENOTSUP) {
-			tst_brkm(TCONF, NULL,
-				 "%s:%d: no xattr support in fs or mounted "
-				 "without user_xattr option", file, lineno);
+			tst_brkm_(file, lineno, TCONF, NULL,
+				"no xattr support in fs or mounted without user_xattr option");
+			return rval;
 		}
 
-		tst_brkm(TBROK | TERRNO, NULL,
-			 "%s:%d: getxattr(%s, %s, %p, %zu) failed",
-			 file, lineno, path, name, value, size);
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+			"getxattr(%s, %s, %p, %zu) failed",
+			path, name, value, size);
+	} else if (rval < 0) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+			"Invalid getxattr(%s, %s, %p, %zu) return value %zd",
+			path, name, value, size, rval);
 	}
 
 	return rval;
@@ -882,16 +1008,20 @@ int safe_setxattr(const char *file, const int lineno, const char *path,
 
 	rval = setxattr(path, name, value, size, flags);
 
-	if (rval) {
+	if (rval == -1) {
 		if (errno == ENOTSUP) {
-			tst_brkm(TCONF, NULL,
-				 "%s:%d: no xattr support in fs or mounted "
-				 "without user_xattr option", file, lineno);
+			tst_brkm_(file, lineno, TCONF, NULL,
+				"no xattr support in fs or mounted without user_xattr option");
+			return rval;
 		}
 
-		tst_brkm(TBROK | TERRNO, NULL,
-			 "%s:%d: setxattr(%s, %s, %p, %zu) failed",
-			 file, lineno, path, name, value, size);
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+			"setxattr(%s, %s, %p, %zu) failed",
+			path, name, value, size);
+	} else if (rval) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+			"Invalid setxattr(%s, %s, %p, %zu) return value %d",
+			path, name, value, size, rval);
 	}
 
 	return rval;
@@ -904,16 +1034,20 @@ int safe_lsetxattr(const char *file, const int lineno, const char *path,
 
 	rval = lsetxattr(path, name, value, size, flags);
 
-	if (rval) {
+	if (rval == -1) {
 		if (errno == ENOTSUP) {
-			tst_brkm(TCONF, NULL,
-				 "%s:%d: no xattr support in fs or mounted "
-				 "without user_xattr option", file, lineno);
+			tst_brkm_(file, lineno, TCONF, NULL,
+				"no xattr support in fs or mounted without user_xattr option");
+			return rval;
 		}
 
-		tst_brkm(TBROK | TERRNO, NULL,
-			 "%s:%d: lsetxattr(%s, %s, %p, %zu, %i) failed",
-			 file, lineno, path, name, value, size, flags);
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+			"lsetxattr(%s, %s, %p, %zu, %i) failed",
+			path, name, value, size, flags);
+	} else if (rval) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+			"Invalid lsetxattr(%s, %s, %p, %zu, %i) return value %d",
+			path, name, value, size, flags, rval);
 	}
 
 	return rval;
@@ -926,16 +1060,20 @@ int safe_fsetxattr(const char *file, const int lineno, int fd, const char *name,
 
 	rval = fsetxattr(fd, name, value, size, flags);
 
-	if (rval) {
+	if (rval == -1) {
 		if (errno == ENOTSUP) {
-			tst_brkm(TCONF, NULL,
-				 "%s:%d: no xattr support in fs or mounted "
-				 "without user_xattr option", file, lineno);
+			tst_brkm_(file, lineno, TCONF, NULL,
+				"no xattr support in fs or mounted without user_xattr option");
+			return rval;
 		}
 
-		tst_brkm(TBROK | TERRNO, NULL,
-			 "%s:%d: fsetxattr(%i, %s, %p, %zu, %i) failed",
-			 file, lineno, fd, name, value, size, flags);
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+			"fsetxattr(%i, %s, %p, %zu, %i) failed",
+			fd, name, value, size, flags);
+	} else if (rval) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+			"Invalid fsetxattr(%i, %s, %p, %zu, %i) return value %d",
+			fd, name, value, size, flags, rval);
 	}
 
 	return rval;
@@ -948,16 +1086,19 @@ int safe_removexattr(const char *file, const int lineno, const char *path,
 
 	rval = removexattr(path, name);
 
-	if (rval) {
+	if (rval == -1) {
 		if (errno == ENOTSUP) {
-			tst_brkm(TCONF, NULL,
-				"%s:%d: no xattr support in fs or mounted "
-				"without user_xattr option", file, lineno);
+			tst_brkm_(file, lineno, TCONF, NULL,
+				"no xattr support in fs or mounted without user_xattr option");
+			return rval;
 		}
 
-		tst_brkm(TBROK | TERRNO, NULL,
-			 "%s:%d: removexattr(%s, %s) failed",
-			 file, lineno, path, name);
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+			"removexattr(%s, %s) failed", path, name);
+	} else if (rval) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+			"Invalid removexattr(%s, %s) return value %d", path,
+			name, rval);
 	}
 
 	return rval;
@@ -970,16 +1111,19 @@ int safe_lremovexattr(const char *file, const int lineno, const char *path,
 
 	rval = lremovexattr(path, name);
 
-	if (rval) {
+	if (rval == -1) {
 		if (errno == ENOTSUP) {
-			tst_brkm(TCONF, NULL,
-				"%s:%d: no xattr support in fs or mounted "
-				"without user_xattr option", file, lineno);
+			tst_brkm_(file, lineno, TCONF, NULL,
+				"no xattr support in fs or mounted without user_xattr option");
+			return rval;
 		}
 
-		tst_brkm(TBROK | TERRNO, NULL,
-			 "%s:%d: lremovexattr(%s, %s) failed",
-			 file, lineno, path, name);
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+			"lremovexattr(%s, %s) failed", path, name);
+	} else if (rval) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+			"Invalid lremovexattr(%s, %s) return value %d", path,
+			name, rval);
 	}
 
 	return rval;
@@ -992,16 +1136,19 @@ int safe_fremovexattr(const char *file, const int lineno, int fd,
 
 	rval = fremovexattr(fd, name);
 
-	if (rval) {
+	if (rval == -1) {
 		if (errno == ENOTSUP) {
-			tst_brkm(TCONF, NULL,
-				"%s:%d: no xattr support in fs or mounted "
-				"without user_xattr option", file, lineno);
+			tst_brkm_(file, lineno, TCONF, NULL,
+				"no xattr support in fs or mounted without user_xattr option");
+			return rval;
 		}
 
-		tst_brkm(TBROK | TERRNO, NULL,
-			 "%s:%d: fremovexattr(%i, %s) failed",
-			 file, lineno, fd, name);
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+			"fremovexattr(%i, %s) failed", fd, name);
+	} else if (rval) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+			"Invalid fremovexattr(%i, %s) return value %d", fd,
+			name, rval);
 	}
 
 	return rval;
@@ -1013,9 +1160,12 @@ int safe_fsync(const char *file, const int lineno, int fd)
 
 	rval = fsync(fd);
 
-	if (rval) {
-		tst_brkm(TBROK | TERRNO, NULL,
-			"%s:%d: fsync(%i) failed", file, lineno, fd);
+	if (rval == -1) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+			"fsync(%i) failed", fd);
+	} else if (rval) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+			"Invalid fsync(%i) return value %d", fd, rval);
 	}
 
 	return rval;
@@ -1026,9 +1176,10 @@ pid_t safe_setsid(const char *file, const int lineno)
 	pid_t rval;
 
 	rval = setsid();
+
 	if (rval == -1) {
-		tst_brkm(TBROK | TERRNO, NULL,
-			 "%s:%d: setsid() failed", file, lineno);
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+			"setsid() failed");
 	}
 
 	return rval;
@@ -1040,9 +1191,13 @@ int safe_mknod(const char *file, const int lineno, const char *pathname,
 	int rval;
 
 	rval = mknod(pathname, mode, dev);
+
 	if (rval == -1) {
-		tst_brkm(TBROK | TERRNO, NULL,
-			 "%s:%d: mknod() failed", file, lineno);
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+			"mknod() failed");
+	} else if (rval) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+			"Invalid mknod() return value %d", rval);
 	}
 
 	return rval;
@@ -1054,9 +1209,13 @@ int safe_mlock(const char *file, const int lineno, const void *addr,
 	int rval;
 
 	rval = mlock(addr, len);
+
 	if (rval == -1) {
-		tst_brkm(TBROK | TERRNO, NULL,
-			 "%s:%d: mlock() failed", file, lineno);
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+			"mlock() failed");
+	} else if (rval) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+			"Invalid mlock() return value %d", rval);
 	}
 
 	return rval;
@@ -1068,9 +1227,13 @@ int safe_munlock(const char *file, const int lineno, const void *addr,
 	int rval;
 
 	rval = munlock(addr, len);
+
 	if (rval == -1) {
-		tst_brkm(TBROK | TERRNO, NULL,
-			 "%s:%d: munlock() failed", file, lineno);
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+			"munlock() failed");
+	} else if (rval) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+			"Invalid munlock() return value %d", rval);
 	}
 
 	return rval;
@@ -1082,9 +1245,13 @@ int safe_mincore(const char *file, const int lineno, void *start,
 	int rval;
 
 	rval = mincore(start, length, vec);
+
 	if (rval == -1) {
-		tst_brkm(TBROK | TERRNO, NULL,
-			 "%s:%d: mincore() failed", file, lineno);
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+			"mincore() failed");
+	} else if (rval) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+			"Invalid mincore() return value %d", rval);
 	}
 
 	return rval;
-- 
2.28.0

[LTP] [PATCH v2 5/7] Unify error handling in lib/safe_net.c

[Martin Doucha]

- Properly format caller file:line location
- Pedantically check invalid syscall return values
- Use safe_*() functions in tst_get_unused_port()

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

Changes since v1:
- Code style fixes

 lib/safe_net.c | 260 +++++++++++++++++++++++++++++--------------------
 1 file changed, 154 insertions(+), 106 deletions(-)

diff --git a/lib/safe_net.c b/lib/safe_net.c
index 499368007..f9ebea610 100644
--- a/lib/safe_net.c
+++ b/lib/safe_net.c
@@ -18,6 +18,7 @@
 
 #include <errno.h>
 #include "test.h"
+#include "safe_macros_fn.h"
 #include "safe_net_fn.h"
 
 char *tst_sock_addr(const struct sockaddr *sa, socklen_t salen, char *res,
@@ -111,7 +112,7 @@ int safe_socket(const char *file, const int lineno, void (cleanup_fn)(void),
 
 	rval = socket(domain, type, protocol);
 
-	if (rval < 0) {
+	if (rval == -1) {
 		switch (errno) {
 		case EPROTONOSUPPORT:
 		case ESOCKTNOSUPPORT:
@@ -124,9 +125,12 @@ int safe_socket(const char *file, const int lineno, void (cleanup_fn)(void),
 			ttype = TBROK;
 		}
 
-		tst_brkm(ttype | TERRNO, cleanup_fn,
-			 "%s:%d: socket(%d, %d, %d) failed", file, lineno,
-			 domain, type, protocol);
+		tst_brkm_(file, lineno, ttype | TERRNO, cleanup_fn,
+			"socket(%d, %d, %d) failed", domain, type, protocol);
+	} else if (rval < 0) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Invalid socket(%d, %d, %d) return value %d", domain,
+			type, protocol, rval);
 	}
 
 	return rval;
@@ -139,7 +143,7 @@ int safe_socketpair(const char *file, const int lineno, int domain, int type,
 
 	rval = socketpair(domain, type, protocol, sv);
 
-	if (rval < 0) {
+	if (rval == -1) {
 		switch (errno) {
 		case EPROTONOSUPPORT:
 		case EOPNOTSUPP:
@@ -150,9 +154,13 @@ int safe_socketpair(const char *file, const int lineno, int domain, int type,
 			ttype = TBROK;
 		}
 
-		tst_brkm(ttype | TERRNO, NULL,
-			 "%s:%d: socketpair(%d, %d, %d, %p) failed",
-			 file, lineno, domain, type, protocol, sv);
+		tst_brkm_(file, lineno, ttype | TERRNO, NULL,
+			"socketpair(%d, %d, %d, %p) failed", domain, type,
+			protocol, sv);
+	} else if (rval) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+			"Invalid socketpair(%d, %d, %d, %p) return value %d",
+			domain, type, protocol, sv, rval);
 	}
 
 	return rval;
@@ -163,12 +171,15 @@ int safe_getsockopt(const char *file, const int lineno, int sockfd, int level,
 {
 	int rval = getsockopt(sockfd, level, optname, optval, optlen);
 
-	if (!rval)
-		return 0;
-
-	tst_brkm(TBROK | TERRNO, NULL,
-		 "%s:%d: getsockopt(%d, %d, %d, %p, %p) failed",
-		 file, lineno, sockfd, level, optname, optval, optlen);
+	if (rval == -1) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+			"getsockopt(%d, %d, %d, %p, %p) failed",
+			sockfd, level, optname, optval, optlen);
+	} else if (rval) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+			"Invalid getsockopt(%d, %d, %d, %p, %p) return value %d",
+			sockfd, level, optname, optval, optlen, rval);
+	}
 
 	return rval;
 }
@@ -180,10 +191,14 @@ int safe_setsockopt(const char *file, const int lineno, int sockfd, int level,
 
 	rval = setsockopt(sockfd, level, optname, optval, optlen);
 
-	if (rval) {
-		tst_brkm(TBROK | TERRNO, NULL,
-			 "%s:%d: setsockopt(%d, %d, %d, %p, %d) failed",
-			 file, lineno, sockfd, level, optname, optval, optlen);
+	if (rval == -1) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+			"setsockopt(%d, %d, %d, %p, %d) failed",
+			sockfd, level, optname, optval, optlen);
+	} else if (rval) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+			"Invalid setsockopt(%d, %d, %d, %p, %d) return value %d",
+			sockfd, level, optname, optval, optlen, rval);
 	}
 
 	return rval;
@@ -197,9 +212,13 @@ ssize_t safe_send(const char *file, const int lineno, char len_strict,
 	rval = send(sockfd, buf, len, flags);
 
 	if (rval == -1 || (len_strict && (size_t)rval != len)) {
-		tst_brkm(TBROK | TERRNO, NULL,
-			 "%s:%d: send(%d, %p, %zu, %d) failed",
-			 file, lineno, sockfd, buf, len, flags);
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+			"send(%d, %p, %zu, %d) failed", sockfd, buf, len,
+			flags);
+	} else if (rval < 0) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+			"Invalid send(%d, %p, %zu, %d) return value %zd",
+			sockfd, buf, len, flags, rval);
 	}
 
 	return rval;
@@ -215,11 +234,17 @@ ssize_t safe_sendto(const char *file, const int lineno, char len_strict,
 	rval = sendto(sockfd, buf, len, flags, dest_addr, addrlen);
 
 	if (rval == -1 || (len_strict && (size_t)rval != len)) {
-		tst_brkm(TBROK | TERRNO, NULL,
-			 "%s:%d: sendto(%d, %p, %zu, %d, %s, %d) failed",
-			 file, lineno, sockfd, buf, len, flags,
-			 tst_sock_addr(dest_addr, addrlen, res, sizeof(res)),
-			 addrlen);
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+			"sendto(%d, %p, %zu, %d, %s, %d) failed",
+			sockfd, buf, len, flags,
+			tst_sock_addr(dest_addr, addrlen, res, sizeof(res)),
+			addrlen);
+	} else if (rval < 0) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+			"Invalid sendto(%d, %p, %zu, %d, %s, %d) return value %zd",
+			sockfd, buf, len, flags,
+			tst_sock_addr(dest_addr, addrlen, res, sizeof(res)),
+			addrlen, rval);
 	}
 
 	return rval;
@@ -233,15 +258,16 @@ ssize_t safe_sendmsg(const char *file, const int lineno, size_t len,
 	rval = sendmsg(sockfd, msg, flags);
 
 	if (rval == -1) {
-		tst_brkm(TBROK | TERRNO, NULL,
-			 "%s:%d: sendmsg(%d, %p, %d) failed",
-			 file, lineno, sockfd, msg, flags);
-	}
-
-	if (len && (size_t)rval != len) {
-		tst_brkm(TBROK, NULL,
-			 "%s:%d: sendmsg(%d, %p, %d) ret(%zd) != len(%zu)",
-			 file, lineno, sockfd, msg, flags, rval, len);
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+			"sendmsg(%d, %p, %d) failed", sockfd, msg, flags);
+	} else if (rval < 0) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+			"Invalid sendmsg(%d, %p, %d) return value %zd",
+			sockfd, msg, flags, rval);
+	} else if (len && (size_t)rval != len) {
+		tst_brkm_(file, lineno, TBROK, NULL,
+			 "sendmsg(%d, %p, %d) ret(%zd) != len(%zu)",
+			 sockfd, msg, flags, rval, len);
 	}
 
 	return rval;
@@ -255,15 +281,16 @@ ssize_t safe_recvmsg(const char *file, const int lineno, size_t len,
 	rval = recvmsg(sockfd, msg, flags);
 
 	if (rval == -1) {
-		tst_brkm(TBROK | TERRNO, NULL,
-			 "%s:%d: recvmsg(%d, %p, %d) failed",
-			 file, lineno, sockfd, msg, flags);
-	}
-
-	if (len && (size_t)rval != len) {
-		tst_brkm(TBROK, NULL,
-			 "%s:%d: recvmsg(%d, %p, %d) ret(%zd) != len(%zu)",
-			 file, lineno, sockfd, msg, flags, rval, len);
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+			"recvmsg(%d, %p, %d) failed", sockfd, msg, flags);
+	} else if (rval < 0) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+			"Invalid recvmsg(%d, %p, %d) return value %zd",
+			sockfd, msg, flags, rval);
+	} else if (len && (size_t)rval != len) {
+		tst_brkm_(file, lineno, TBROK, NULL,
+			"recvmsg(%d, %p, %d) ret(%zd) != len(%zu)",
+			sockfd, msg, flags, rval, len);
 	}
 
 	return rval;
@@ -274,35 +301,41 @@ int safe_bind(const char *file, const int lineno, void (cleanup_fn)(void),
 	      int socket, const struct sockaddr *address,
 	      socklen_t address_len)
 {
-	int i;
+	int i, ret;
 	char buf[128];
 
 	for (i = 0; i < 120; i++) {
-		if (!bind(socket, address, address_len))
+		ret = bind(socket, address, address_len);
+
+		if (!ret)
 			return 0;
 
-		if (errno != EADDRINUSE) {
-			tst_brkm(TBROK | TERRNO, cleanup_fn,
-				 "%s:%d: bind(%d, %s, %d) failed", file, lineno,
-				 socket, tst_sock_addr(address, address_len,
-						       buf, sizeof(buf)),
-				 address_len);
-			return -1;
+		if (ret != -1) {
+			tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+				"Invalid bind(%d, %s, %d) return value %d",
+				socket, tst_sock_addr(address, address_len,
+				buf, sizeof(buf)), address_len, ret);
+			return ret;
+		} else if (errno != EADDRINUSE) {
+			tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+				"bind(%d, %s, %d) failed", socket,
+				tst_sock_addr(address, address_len, buf,
+				sizeof(buf)), address_len);
+			return ret;
 		}
 
 		if ((i + 1) % 10 == 0) {
-			tst_resm(TINFO, "address is in use, waited %3i sec",
-				 i + 1);
+			tst_resm_(file, lineno, TINFO,
+				"address is in use, waited %3i sec", i + 1);
 		}
 
 		sleep(1);
 	}
 
-	tst_brkm(TBROK | TERRNO, cleanup_fn,
-		 "%s:%d: Failed to bind(%d, %s, %d) after 120 retries", file,
-		 lineno, socket,
-		 tst_sock_addr(address, address_len, buf, sizeof(buf)),
-		 address_len);
+	tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+		"Failed to bind(%d, %s, %d) after 120 retries", socket,
+		tst_sock_addr(address, address_len, buf, sizeof(buf)),
+		address_len);
 	return -1;
 }
 
@@ -313,10 +346,13 @@ int safe_listen(const char *file, const int lineno, void (cleanup_fn)(void),
 
 	rval = listen(socket, backlog);
 
-	if (rval < 0) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "%s:%d: listen(%d, %d) failed", file, lineno, socket,
-			 backlog);
+	if (rval == -1) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"listen(%d, %d) failed", socket, backlog);
+	} else if (rval) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Invalid listen(%d, %d) return value %d", socket,
+			backlog, rval);
 	}
 
 	return rval;
@@ -329,10 +365,13 @@ int safe_accept(const char *file, const int lineno, void (cleanup_fn)(void),
 
 	rval = accept(sockfd, addr, addrlen);
 
-	if (rval < 0) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			"%s:%d: accept(%d, %p, %d) failed", file, lineno,
-			sockfd, addr, *addrlen);
+	if (rval == -1) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"accept(%d, %p, %d) failed", sockfd, addr, *addrlen);
+	} else if (rval < 0) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Invalid accept(%d, %p, %d) return value %d", sockfd,
+			addr, *addrlen, rval);
 	}
 
 	return rval;
@@ -346,11 +385,16 @@ int safe_connect(const char *file, const int lineno, void (cleanup_fn)(void),
 
 	rval = connect(sockfd, addr, addrlen);
 
-	if (rval < 0) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "%s:%d: connect(%d, %s, %d) failed", file, lineno,
-			 sockfd, tst_sock_addr(addr, addrlen, buf,
-					       sizeof(buf)), addrlen);
+	if (rval == -1) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"connect(%d, %s, %d) failed", sockfd,
+			tst_sock_addr(addr, addrlen, buf, sizeof(buf)),
+			addrlen);
+	} else if (rval) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Invalid connect(%d, %s, %d) return value %d", sockfd,
+			tst_sock_addr(addr, addrlen, buf, sizeof(buf)),
+			addrlen, rval);
 	}
 
 	return rval;
@@ -365,11 +409,16 @@ int safe_getsockname(const char *file, const int lineno,
 
 	rval = getsockname(sockfd, addr, addrlen);
 
-	if (rval < 0) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "%s:%d: getsockname(%d, %s, %d) failed", file, lineno,
-			 sockfd, tst_sock_addr(addr, *addrlen, buf,
-					       sizeof(buf)), *addrlen);
+	if (rval == -1) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"getsockname(%d, %s, %d) failed", sockfd,
+			tst_sock_addr(addr, *addrlen, buf, sizeof(buf)),
+			*addrlen);
+	} else if (rval) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Invalid getsockname(%d, %s, %d) return value %d",
+			sockfd, tst_sock_addr(addr, *addrlen, buf,
+			sizeof(buf)), *addrlen, rval);
 	}
 
 	return rval;
@@ -380,10 +429,13 @@ int safe_gethostname(const char *file, const int lineno,
 {
 	int rval = gethostname(name, size);
 
-	if (rval < 0) {
-		tst_brkm(TBROK | TERRNO, NULL,
-			 "%s:%d: gethostname(%p, %zu) failed",
-			 file, lineno, name, size);
+	if (rval == -1) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+			"gethostname(%p, %zu) failed", name, size);
+	} else if (rval) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+			"Invalid gethostname(%p, %zu) return value %d", name,
+			size, rval);
 	}
 
 	return rval;
@@ -395,7 +447,7 @@ int safe_gethostname(const char *file, const int lineno,
 unsigned short tst_get_unused_port(const char *file, const int lineno,
 	      void (cleanup_fn)(void), unsigned short family, int type)
 {
-	int sock;
+	int sock, ret;
 	socklen_t slen;
 	struct sockaddr_storage _addr;
 	struct sockaddr *addr = (struct sockaddr *)&_addr;
@@ -418,35 +470,31 @@ unsigned short tst_get_unused_port(const char *file, const int lineno,
 		break;
 
 	default:
-		tst_brkm(TBROK, cleanup_fn,
-			"%s:%d: unknown family", file, lineno);
+		tst_brkm_(file, lineno, TBROK, cleanup_fn,
+			"%s(): Unsupported socket family %d", __func__,
+			family);
 		return -1;
 	}
 
-	sock = socket(addr->sa_family, type, 0);
-	if (sock < 0) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "%s:%d: socket failed", file, lineno);
-		return -1;
-	}
+	sock = safe_socket(file, lineno, cleanup_fn, addr->sa_family, type, 0);
 
-	if (bind(sock, addr, slen) < 0) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "%s:%d: bind failed", file, lineno);
-		return -1;
-	}
+	if (sock < 0)
+		return sock;
 
-	if (getsockname(sock, addr, &slen) == -1) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "%s:%d: getsockname failed", file, lineno);
-		return -1;
-	}
+	ret = safe_bind(file, lineno, cleanup_fn, sock, addr, slen);
 
-	if (close(sock) == -1) {
-		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			 "%s:%d: close failed", file, lineno);
-		return -1;
-	}
+	if (ret)
+		return ret;
+
+	ret = safe_getsockname(file, lineno, cleanup_fn, sock, addr, &slen);
+
+	if (ret)
+		return ret;
+
+	ret = safe_close(file, lineno, cleanup_fn, sock);
+
+	if (ret)
+		return ret;
 
 	switch (family) {
 	case AF_INET:
-- 
2.28.0

[LTP] [PATCH v2 6/7] Unify error handling in lib/tst_fs_setup.c

[Martin Doucha]

- Properly format caller file:line location

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

Changes since v1:
- Replaced the last remaining tst_brk() with tst_brk_()

 lib/tst_fs_setup.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/lib/tst_fs_setup.c b/lib/tst_fs_setup.c
index 54ea37077..6b93483de 100644
--- a/lib/tst_fs_setup.c
+++ b/lib/tst_fs_setup.c
@@ -36,14 +36,15 @@ int mount_overlay(const char *file, const int lineno, int skip)
 
 	if (errno == ENODEV) {
 		if (skip) {
-			tst_brk(TCONF, "%s:%d: " TST_FS_SETUP_OVERLAYFS_MSG,
-				file, lineno);
+			tst_brk_(file, lineno, TCONF,
+				TST_FS_SETUP_OVERLAYFS_MSG);
 		} else {
-			tst_res(TINFO, "%s:%d: " TST_FS_SETUP_OVERLAYFS_MSG,
-				file, lineno);
+			tst_res_(file, lineno, TINFO,
+				TST_FS_SETUP_OVERLAYFS_MSG);
 		}
 	} else {
-		tst_brk(TBROK | TERRNO, "overlayfs mount failed");
+		tst_brk_(file, lineno, TBROK | TERRNO,
+			"overlayfs mount failed");
 	}
 	return ret;
 }
-- 
2.28.0

[LTP] [PATCH v2 7/7] Unify error handling in include/lapi/safe_rt_signal.h

[Martin Doucha]

- Properly format caller file:line location
- Pedantically check invalid syscall return values

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

Changes since v1:
- Code style fixes

 include/lapi/safe_rt_signal.h | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/include/lapi/safe_rt_signal.h b/include/lapi/safe_rt_signal.h
index 67fa44417..952085608 100644
--- a/include/lapi/safe_rt_signal.h
+++ b/include/lapi/safe_rt_signal.h
@@ -15,10 +15,15 @@ static inline int safe_rt_sigaction(const char *file, const int lineno,
 	int ret;
 
 	ret = ltp_rt_sigaction(signum, act, oact, sigsetsize);
-	if (ret < 0) {
-		tst_brk(TBROK | TERRNO,
-			"%s:%d: ltp_rt_sigaction(%i, %p, %p, %zu) failed",
-			file, lineno, signum, act, oact, sigsetsize);
+
+	if (ret == -1) {
+		tst_brk_(file, lineno, TBROK | TERRNO,
+			"ltp_rt_sigaction(%i, %p, %p, %zu) failed",
+			signum, act, oact, sigsetsize);
+	} else if (ret) {
+		tst_brk_(file, lineno, TBROK | TERRNO,
+			"Invalid ltp_rt_sigaction(%i, %p, %p, %zu) return value %d",
+			signum, act, oact, sigsetsize, ret);
 	}
 
 	return ret;
@@ -35,10 +40,14 @@ static inline int safe_rt_sigprocmask(const char *file, const int lineno,
 	int ret;
 
 	ret = tst_syscall(__NR_rt_sigprocmask, how, set, oldset, sigsetsize);
-	if (ret < 0) {
-		tst_brk(TBROK | TERRNO,
-		        "%s:%d: rt_sigprocmask(%i, %p, %p, %zu) failed",
-			file, lineno, how, set, oldset, sigsetsize);
+	if (ret == -1) {
+		tst_brk_(file, lineno, TBROK | TERRNO,
+			"rt_sigprocmask(%i, %p, %p, %zu) failed",
+			how, set, oldset, sigsetsize);
+	} else if (ret) {
+		tst_brk_(file, lineno, TBROK | TERRNO,
+			"Invalid rt_sigprocmask(%i, %p, %p, %zu) return value %d",
+			how, set, oldset, sigsetsize, ret);
 	}
 
 	return ret;
-- 
2.28.0

[LTP] [PATCH v2 1/7] Unify error handling in lib/tst_safe_sysv_ipc.c

[Cyril Hrubis]

Hi!
Pushed, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2 3/7] Unify error handling in lib/safe_file_ops.c

[Cyril Hrubis]

Hi!
Pushed, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2 4/7] Unify error handling in lib/safe_macros.c

[Cyril Hrubis]

Hi!
Pushed, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2 5/7] Unify error handling in lib/safe_net.c

[Cyril Hrubis]

Hi!
Pushed, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2 6/7] Unify error handling in lib/tst_fs_setup.c

[Cyril Hrubis]

Hi!
Pushed, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2 7/7] Unify error handling in include/lapi/safe_rt_signal.h

[Cyril Hrubis]

Hi!
Pushed, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz