[LTP] [PATCH 1/2] safe_stdio: More checks for invalid function return values

public inbox for ltp@lists.linux.it
 help / color / mirror / Atom feed

* [LTP] [PATCH 1/2] safe_stdio: More checks for invalid function return values
@ 2026-04-17  8:47 Petr Vorel
  2026-04-17  8:47 ` [LTP] [PATCH 2/2] lib: Be pedantic on invalid comparison check Petr Vorel
  2026-04-17  9:24 ` [LTP] safe_stdio: More checks for invalid function return values linuxtestproject.agent
  0 siblings, 2 replies; 4+ messages in thread
From: Petr Vorel @ 2026-04-17  8:47 UTC (permalink / raw)
  To: ltp

LTP library pedantically check invalid function return values, add
missing ones.

Fixes: 56221c5bb2 ("lib: add safe macros for stream testing suite")
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 lib/safe_stdio.c | 30 +++++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/lib/safe_stdio.c b/lib/safe_stdio.c
index feb8a4b5c8..ed5bca0072 100644
--- a/lib/safe_stdio.c
+++ b/lib/safe_stdio.c
@@ -153,7 +153,11 @@ int safe_fseek(const char *file, const int lineno,
 
 	if (ret == -1) {
 		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
-			"fseek(%p, %ld, %d)", f, offset, whence);
+			"fseek(%p, %ld, %d) failed", f, offset, whence);
+	} else if (ret < 0) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+			"Invalid fseek(%p, %ld, %d) return value %d",
+			f, offset, whence, ret);
 	}
 
 	return ret;
@@ -167,8 +171,12 @@ long safe_ftell(const char *file, const int lineno,
 	errno = 0;
 	ret = ftell(f);
 
-	if (ret == -1)
-		tst_brkm_(file, lineno, TBROK | TERRNO, NULL, "ftell(%p)", f);
+	if (ret == -1) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL, "ftell(%p) failed", f);
+	} else if (ret < 0) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+				  "Invalid ftell(%p) return value %ld", f, ret);
+	}
 
 	return ret;
 }
@@ -181,8 +189,12 @@ int safe_fileno(const char *file, const int lineno,
 	errno = 0;
 	ret = fileno(f);
 
-	if (ret == -1)
-		tst_brkm_(file, lineno, TBROK | TERRNO, NULL, "fileno(%p)", f);
+	if (ret == -1) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL, "fileno(%p) failed", f);
+	} else if (ret < 0) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+				  "Invalid fileno(%p) return value %d", f, ret);
+	}
 
 	return ret;
 }
@@ -195,8 +207,12 @@ int safe_fflush(const char *file, const int lineno,
 	errno = 0;
 	ret = fflush(f);
 
-	if (ret == EOF)
-		tst_brkm_(file, lineno, TBROK | TERRNO, NULL, "fflush(%p)", f);
+	if (ret == EOF) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL, "fflush(%p) failed", f);
+	} else if (!ret) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+				  "Invalid fflush(%p) return value %d", f, ret);
+	}
 
 	return ret;
 }
-- 
2.53.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [LTP] [PATCH 2/2] lib: Be pedantic on invalid comparison check
  2026-04-17  8:47 [LTP] [PATCH 1/2] safe_stdio: More checks for invalid function return values Petr Vorel
@ 2026-04-17  8:47 ` Petr Vorel
  2026-04-17  9:24 ` [LTP] safe_stdio: More checks for invalid function return values linuxtestproject.agent
  1 sibling, 0 replies; 4+ messages in thread
From: Petr Vorel @ 2026-04-17  8:47 UTC (permalink / raw)
  To: ltp

Invalid return values are when "ret < -1". While "ret < 0" mostly works
due previous check "ret == -1" it can have misleading message when after
code refactoring.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 lib/safe_file_ops.c     |  2 +-
 lib/safe_macros.c       | 16 ++++++++--------
 lib/safe_net.c          | 14 +++++++-------
 lib/safe_stdio.c        |  4 ++--
 lib/tst_safe_io_uring.c |  4 ++--
 lib/tst_safe_macros.c   | 16 ++++++++--------
 lib/tst_safe_sysv_ipc.c | 10 +++++-----
 lib/tst_safe_timerfd.c  |  2 +-
 lib/tst_security.c      |  2 +-
 9 files changed, 35 insertions(+), 35 deletions(-)

diff --git a/lib/safe_file_ops.c b/lib/safe_file_ops.c
index 8314c4b1bb..0982274120 100644
--- a/lib/safe_file_ops.c
+++ b/lib/safe_file_ops.c
@@ -361,7 +361,7 @@ int safe_touch(const char *file, const int lineno,
 		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
 			"Failed to open file '%s'", pathname);
 		return ret;
-	} else if (ret < 0) {
+	} else if (ret < -1) {
 		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
 			"Invalid open(%s) return value %d", pathname, ret);
 		return ret;
diff --git a/lib/safe_macros.c b/lib/safe_macros.c
index 68b8747b4d..22b321fc45 100644
--- a/lib/safe_macros.c
+++ b/lib/safe_macros.c
@@ -89,7 +89,7 @@ safe_creat(const char *file, const int lineno, void (*cleanup_fn) (void),
 	if (rval == -1) {
 		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
 			"creat(%s,%04o) failed", pathname, mode);
-	} else if (rval < 0) {
+	} else if (rval < -1) {
 		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
 			"Invalid creat(%s,%04o) return value %d", pathname,
 			mode, rval);
@@ -258,7 +258,7 @@ int safe_open(const char *file, const int lineno, void (*cleanup_fn) (void),
 	if (rval == -1) {
 		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
 			"open(%s,%d,%04o) failed", pathname, oflags, mode);
-	} else if (rval < 0) {
+	} else if (rval < -1) {
 		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
 			"Invalid open(%s,%d,%04o) return value %d", pathname,
 			oflags, mode, rval);
@@ -297,7 +297,7 @@ ssize_t safe_read(const char *file, const int lineno, void (*cleanup_fn) (void),
 		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
 			"read(%d,%p,%zu) failed, returned %zd", fildes, buf,
 			nbyte, rval);
-	} else if (rval < 0) {
+	} else if (rval < -1) {
 		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
 			"Invalid read(%d,%p,%zu) return value %zd", fildes,
 			buf, nbyte, rval);
@@ -495,7 +495,7 @@ ssize_t safe_readlink(const char *file, const int lineno,
 	if (rval == -1) {
 		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
 			"readlink(%s,%p,%zu) failed", path, buf, bufsize);
-	} else if (rval < 0) {
+	} else if (rval < -1) {
 		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
 			"Invalid readlink(%s,%p,%zu) return value %zd", path,
 			buf, bufsize, rval);
@@ -554,7 +554,7 @@ ssize_t safe_write(const char *file, const int lineno, void (cleanup_fn) (void),
 			return rval;
 		}
 
-		if (rval < 0) {
+		if (rval < -1) {
 			tst_brkm_(file, lineno, TBROK, cleanup_fn,
 			          "invalid write() return value %zi",
 				  rval);
@@ -804,7 +804,7 @@ pid_t safe_wait(const char *file, const int lineno, void (cleanup_fn)(void),
 	if (rval == -1) {
 		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
 			"wait(%p) failed", status);
-	} else if (rval < 0) {
+	} else if (rval < -1) {
 		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
 			"Invalid wait(%p) return value %d", status, rval);
 	}
@@ -822,7 +822,7 @@ pid_t safe_waitpid(const char *file, const int lineno, void (cleanup_fn)(void),
 	if (rval == -1) {
 		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
 			"waitpid(%d,%p,%d) failed", pid, status, opts);
-	} else if (rval < 0) {
+	} else if (rval < -1) {
 		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
 			"Invalid waitpid(%d,%p,%d) return value %d", pid,
 			status, opts, rval);
@@ -1116,7 +1116,7 @@ ssize_t safe_getxattr(const char *file, const int lineno, const char *path,
 		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
 			"getxattr(%s, %s, %p, %zu) failed",
 			path, name, value, size);
-	} else if (rval < 0) {
+	} else if (rval < -1) {
 		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
 			"Invalid getxattr(%s, %s, %p, %zu) return value %zd",
 			path, name, value, size, rval);
diff --git a/lib/safe_net.c b/lib/safe_net.c
index 5dec0de113..37a980fae9 100644
--- a/lib/safe_net.c
+++ b/lib/safe_net.c
@@ -127,7 +127,7 @@ int safe_socket(const char *file, const int lineno, void (cleanup_fn)(void),
 
 		tst_brkm_(file, lineno, ttype | TERRNO, cleanup_fn,
 			"socket(%d, %d, %d) failed", domain, type, protocol);
-	} else if (rval < 0) {
+	} else if (rval < -1) {
 		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
 			"Invalid socket(%d, %d, %d) return value %d", domain,
 			type, protocol, rval);
@@ -215,7 +215,7 @@ ssize_t safe_send(const char *file, const int lineno, char len_strict,
 		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
 			"send(%d, %p, %zu, %d) failed", sockfd, buf, len,
 			flags);
-	} else if (rval < 0) {
+	} else if (rval < -1) {
 		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
 			"Invalid send(%d, %p, %zu, %d) return value %zd",
 			sockfd, buf, len, flags, rval);
@@ -239,7 +239,7 @@ ssize_t safe_sendto(const char *file, const int lineno, char len_strict,
 			sockfd, buf, len, flags,
 			tst_sock_addr(dest_addr, addrlen, res, sizeof(res)),
 			addrlen);
-	} else if (rval < 0) {
+	} else if (rval < -1) {
 		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
 			"Invalid sendto(%d, %p, %zu, %d, %s, %d) return value %zd",
 			sockfd, buf, len, flags,
@@ -260,7 +260,7 @@ ssize_t safe_sendmsg(const char *file, const int lineno, size_t len,
 	if (rval == -1) {
 		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
 			"sendmsg(%d, %p, %d) failed", sockfd, msg, flags);
-	} else if (rval < 0) {
+	} else if (rval < -1) {
 		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
 			"Invalid sendmsg(%d, %p, %d) return value %zd",
 			sockfd, msg, flags, rval);
@@ -284,7 +284,7 @@ ssize_t safe_recv(const char *file, const int lineno, size_t len,
 		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
 			"recv(%d, %p, %zu, %d) failed", sockfd, buf, size,
 			flags);
-	} else if (rval < 0) {
+	} else if (rval < -1) {
 		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
 			"Invalid recv(%d, %p, %zu, %d) return value %zd",
 			sockfd, buf, size, flags, rval);
@@ -308,7 +308,7 @@ ssize_t safe_recvmsg(const char *file, const int lineno, size_t len,
 	if (rval == -1) {
 		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
 			"recvmsg(%d, %p, %d) failed", sockfd, msg, flags);
-	} else if (rval < 0) {
+	} else if (rval < -1) {
 		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
 			"Invalid recvmsg(%d, %p, %d) return value %zd",
 			sockfd, msg, flags, rval);
@@ -396,7 +396,7 @@ int safe_accept(const char *file, const int lineno, void (cleanup_fn)(void),
 	if (rval == -1) {
 		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
 			"accept(%d, %p, %d) failed", sockfd, addr, *addrlen);
-	} else if (rval < 0) {
+	} else if (rval < -1) {
 		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
 			"Invalid accept(%d, %p, %d) return value %d", sockfd,
 			addr, *addrlen, rval);
diff --git a/lib/safe_stdio.c b/lib/safe_stdio.c
index ed5bca0072..27b38aadcc 100644
--- a/lib/safe_stdio.c
+++ b/lib/safe_stdio.c
@@ -67,7 +67,7 @@ int safe_asprintf(const char *file, const int lineno, void (cleanup_fn)(void),
 	if (ret == -1) {
 		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
 			"asprintf(%s,...) failed", fmt);
-	} else if (ret < 0) {
+	} else if (ret < -1) {
 		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
 			"Invalid asprintf(%s,...) return value %d", fmt, ret);
 	}
@@ -154,7 +154,7 @@ int safe_fseek(const char *file, const int lineno,
 	if (ret == -1) {
 		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
 			"fseek(%p, %ld, %d) failed", f, offset, whence);
-	} else if (ret < 0) {
+	} else if (ret < -1) {
 		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
 			"Invalid fseek(%p, %ld, %d) return value %d",
 			f, offset, whence, ret);
diff --git a/lib/tst_safe_io_uring.c b/lib/tst_safe_io_uring.c
index de6869f501..627cedfe02 100644
--- a/lib/tst_safe_io_uring.c
+++ b/lib/tst_safe_io_uring.c
@@ -21,7 +21,7 @@ int safe_io_uring_init(const char *file, const int lineno,
 		tst_brk_(file, lineno, TBROK | TERRNO,
 			"io_uring_setup() failed");
 		return uring->fd;
-	} else if (uring->fd < 0) {
+	} else if (uring->fd < -1) {
 		tst_brk_(file, lineno, TBROK | TERRNO,
 			"io_uring_setup() returned invalid value %d",
 			uring->fd);
@@ -98,7 +98,7 @@ int safe_io_uring_enter(const char *file, const int lineno, int strict,
 	if (ret == -1) {
 		tst_brk_(file, lineno, TBROK | TERRNO,
 			"io_uring_enter() failed");
-	} else if (ret < 0) {
+	} else if (ret < -1) {
 		tst_brk_(file, lineno, TBROK | TERRNO,
 			"Invalid io_uring_enter() return value %d", ret);
 	} else if (strict && to_submit != (unsigned int)ret) {
diff --git a/lib/tst_safe_macros.c b/lib/tst_safe_macros.c
index cdc8c7dd33..a893f73231 100644
--- a/lib/tst_safe_macros.c
+++ b/lib/tst_safe_macros.c
@@ -68,7 +68,7 @@ pid_t safe_getpgid(const char *file, const int lineno, pid_t pid)
 	if (pgid == -1) {
 		tst_brk_(file, lineno, TBROK | TERRNO, "getpgid(%i) failed",
 			pid);
-	} else if (pgid < 0) {
+	} else if (pgid < -1) {
 		tst_brk_(file, lineno, TBROK | TERRNO,
 			"Invalid getpgid(%i) return value %d", pid, pgid);
 	}
@@ -103,7 +103,7 @@ int safe_getgroups(const char *file, const int lineno, int size, gid_t list[])
 	if (rval == -1) {
 		tst_brk_(file, lineno, TBROK | TERRNO,
 			"getgroups(%i, %p)", size, list);
-	} else if (rval < 0) {
+	} else if (rval < -1) {
 		tst_brk_(file, lineno, TBROK | TERRNO,
 			"Invalid getgroups(%i, %p) return value %d", size,
 			list, rval);
@@ -120,7 +120,7 @@ int safe_personality(const char *filename, unsigned int lineno,
 	if (prev_persona == -1) {
 		tst_brk_(filename, lineno, TBROK | TERRNO,
 			 "persona(%ld) failed", persona);
-	} else if (prev_persona < 0) {
+	} else if (prev_persona < -1) {
 		tst_brk_(filename, lineno, TBROK | TERRNO,
 			 "Invalid persona(%ld) return value %d", persona,
 			 prev_persona);
@@ -139,7 +139,7 @@ int safe_pidfd_open(const char *file, const int lineno, pid_t pid,
 	if (rval == -1) {
 		tst_brk_(file, lineno, TBROK | TERRNO,
 			 "pidfd_open(%i, %i) failed", pid, flags);
-	} else if (rval < 0) {
+	} else if (rval < -1) {
 		tst_brk_(file, lineno, TBROK | TERRNO,
 			 "Invalid pidfd_open(%i, %i) return value %d",
 			 pid, flags, rval);
@@ -521,7 +521,7 @@ int safe_dup(const char *file, const int lineno, int oldfd)
 	if (rval == -1) {
 		tst_brk_(file, lineno, TBROK | TERRNO,
 			"dup(%i) failed", oldfd);
-	} else if (rval < 0) {
+	} else if (rval < -1) {
 		tst_brk_(file, lineno, TBROK | TERRNO,
 			"Invalid dup(%i) return value %d", oldfd, rval);
 	}
@@ -723,7 +723,7 @@ int safe_prctl(const char *file, const int lineno,
 		tst_brk_(file, lineno, TBROK | TERRNO,
 			"prctl(%d, %lu, %lu, %lu, %lu)",
 			option, arg2, arg3, arg4, arg5);
-	} else if (rval < 0) {
+	} else if (rval < -1) {
 		tst_brk_(file, lineno, TBROK | TERRNO,
 			"Invalid prctl(%d, %lu, %lu, %lu, %lu) return value %d",
 			option, arg2, arg3, arg4, arg5, rval);
@@ -746,7 +746,7 @@ ssize_t safe_readv(const char *file, const int lineno, char len_strict,
 	if (rval == -1 || (len_strict && rval != nbyte)) {
 		tst_brk_(file, lineno, TBROK | TERRNO,
 			"readv(%d,%p,%d) failed", fildes, iov, iovcnt);
-	} else if (rval < 0) {
+	} else if (rval < -1) {
 		tst_brk_(file, lineno, TBROK | TERRNO,
 			"Invalid readv(%d,%p,%d) return value %zd",
 			fildes, iov, iovcnt, rval);
@@ -788,7 +788,7 @@ ssize_t safe_writev(const char *file, const int lineno, char len_strict,
 	if (rval == -1 || (len_strict && rval != nbyte)) {
 		tst_brk_(file, lineno, TBROK | TERRNO,
 			"writev(%d,%p,%d) failed", fildes, iov, iovcnt);
-	} else if (rval < 0) {
+	} else if (rval < -1) {
 		tst_brk_(file, lineno, TBROK | TERRNO,
 			"Invalid writev(%d,%p,%d) return value %zd",
 			fildes, iov, iovcnt, rval);
diff --git a/lib/tst_safe_sysv_ipc.c b/lib/tst_safe_sysv_ipc.c
index a196fc9ce4..4a16a86e12 100644
--- a/lib/tst_safe_sysv_ipc.c
+++ b/lib/tst_safe_sysv_ipc.c
@@ -70,7 +70,7 @@ int safe_msgget(const char *file, const int lineno, key_t key, int msgflg)
 	if (rval == -1) {
 		tst_brk_(file, lineno, TBROK | TERRNO, "msgget(%i, %x) failed",
 			(int)key, msgflg);
-	} else if (rval < 0) {
+	} else if (rval < -1) {
 		tst_brk_(file, lineno, TBROK | TERRNO,
 			"Invalid msgget(%i, %x) return value %d", (int)key,
 			msgflg, rval);
@@ -110,7 +110,7 @@ ssize_t safe_msgrcv(const char *file, const int lineno, int msqid, void *msgp,
 		tst_brk_(file, lineno, TBROK | TERRNO,
 			"msgrcv(%i, %p, %zu, %li, %x) failed",
 			msqid, msgp, msgsz, msgtyp, msgflg);
-	} else if (rval < 0) {
+	} else if (rval < -1) {
 		tst_brk_(file, lineno, TBROK | TERRNO,
 			"Invalid msgrcv(%i, %p, %zu, %li, %x) return value %ld",
 			msqid, msgp, msgsz, msgtyp, msgflg, rval);
@@ -148,7 +148,7 @@ int safe_shmget(const char *file, const int lineno, key_t key, size_t size,
 	if (rval == -1) {
 		tst_brk_(file, lineno, TBROK | TERRNO,
 			"shmget(%i, %zu, %x) failed", (int)key, size, shmflg);
-	} else if (rval < 0) {
+	} else if (rval < -1) {
 		tst_brk_(file, lineno, TBROK | TERRNO,
 			"Invalid shmget(%i, %zu, %x) return value %d",
 			(int)key, size, shmflg, rval);
@@ -218,7 +218,7 @@ int safe_semget(const char *file, const int lineno, key_t key, int nsems,
 	if (rval == -1) {
 		tst_brk_(file, lineno, TBROK | TERRNO,
 			"semget(%i, %i, %x) failed", (int)key, nsems, semflg);
-	} else if (rval < 0) {
+	} else if (rval < -1) {
 		tst_brk_(file, lineno, TBROK | TERRNO,
 			"Invalid semget(%i, %i, %x) return value %d",
 			(int)key, nsems, semflg, rval);
@@ -272,7 +272,7 @@ int safe_semop(const char *file, const int lineno, int semid, struct sembuf *sop
 	if (rval == -1) {
 		tst_brk_(file, lineno, TBROK | TERRNO,
 			"semop(%d, %p, %zu) failed", semid, sops, nsops);
-	} else if (rval < 0) {
+	} else if (rval < -1) {
 		tst_brk_(file, lineno, TBROK | TERRNO,
 			"Invalid semop(%d, %p, %zu) return value %d",
 			semid, sops, nsops, rval);
diff --git a/lib/tst_safe_timerfd.c b/lib/tst_safe_timerfd.c
index d31f6e35e5..c08b468fce 100644
--- a/lib/tst_safe_timerfd.c
+++ b/lib/tst_safe_timerfd.c
@@ -21,7 +21,7 @@ int safe_timerfd_create(const char *file, const int lineno,
 	if (fd == -1) {
 		tst_brk_(file, lineno, TTYPE | TERRNO,
 			"timerfd_create(%s) failed", tst_clock_name(clockid));
-	} else if (fd < 0) {
+	} else if (fd < -1) {
 		tst_brk_(file, lineno, TBROK | TERRNO,
 			"Invalid timerfd_create(%s) return value %d",
 			tst_clock_name(clockid), fd);
diff --git a/lib/tst_security.c b/lib/tst_security.c
index 00be4d72e2..b25605377c 100644
--- a/lib/tst_security.c
+++ b/lib/tst_security.c
@@ -122,7 +122,7 @@ int tst_secureboot_enabled(void)
 		tst_res(TINFO | TERRNO,
 			"Cannot open SecureBoot file");
 		return -1;
-	} else if (fd < 0) {
+	} else if (fd < -1) {
 		tst_brk(TBROK | TERRNO, "Invalid open() return value %d", fd);
 		return -1;
 	}
-- 
2.53.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [LTP] safe_stdio: More checks for invalid function return values
  2026-04-17  8:47 [LTP] [PATCH 1/2] safe_stdio: More checks for invalid function return values Petr Vorel
  2026-04-17  8:47 ` [LTP] [PATCH 2/2] lib: Be pedantic on invalid comparison check Petr Vorel
@ 2026-04-17  9:24 ` linuxtestproject.agent
  2026-04-17  9:42   ` Petr Vorel
  1 sibling, 1 reply; 4+ messages in thread
From: linuxtestproject.agent @ 2026-04-17  9:24 UTC (permalink / raw)
  To: Petr Vorel; +Cc: ltp

Hi Petr,

--- [PATCH 1/2] ---

On 2026-04-17, Petr Vorel wrote:
> safe_stdio: More checks for invalid function return values

> -	if (ret == EOF)
> -		tst_brkm_(file, lineno, TBROK | TERRNO, NULL, "fflush(%p)", f);
> +	if (ret == EOF) {
> +		tst_brkm_(file, lineno, TBROK | TERRNO, NULL, "fflush(%p) failed", f);
> +	} else if (!ret) {
> +		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
> +				  "Invalid fflush(%p) return value %d", f, ret);
> +	}

!ret is ret == 0, which is fflush()'s success return — this breaks every
caller on success. fflush() only returns 0 or EOF per POSIX; there is no
invalid return value to guard against, so drop the else if branch entirely.

> +	} else if (ret < 0) {
> +		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
> +				  "Invalid ftell(%p) return value %ld", f, ret);

> +	} else if (ret < 0) {
> +		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
> +				  "Invalid fileno(%p) return value %d", f, ret);

Use ret < -1 here to be consistent with safe_fseek() in the same file and
with the policy established by patch 2/2; also fix the continuation-line
indentation (tabs only, no mixed spaces).

--- [PATCH 2/2] ---

On 2026-04-17, Petr Vorel wrote:
> lib: Be pedantic on invalid comparison check

> diff --git a/lib/safe_stdio.c b/lib/safe_stdio.c
> @@ -67,7 +67,7 @@ int safe_asprintf(...)
> -	} else if (ret < 0) {
> +	} else if (ret < -1) {
> ...
> @@ -154,7 +154,7 @@ int safe_fseek(...)
> -	} else if (ret < 0) {
> +	} else if (ret < -1) {

This fixes asprintf and fseek but misses safe_ftell() and safe_fileno()
from patch 1/2, which are left with ret < 0. Either fix them here or
swap the patch order so new checks are written with ret < -1 from the start.

---
Note:

Our agent completed the review of the patch. The full review can be found
in the conversation context above.

The agent can sometimes produce false positives although often its findings
are genuine. If you find issues with the review, please comment this email
or ignore the suggestions.

Regards,
LTP AI Reviewer

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [LTP] safe_stdio: More checks for invalid function return values
  2026-04-17  9:24 ` [LTP] safe_stdio: More checks for invalid function return values linuxtestproject.agent
@ 2026-04-17  9:42   ` Petr Vorel
  0 siblings, 0 replies; 4+ messages in thread
From: Petr Vorel @ 2026-04-17  9:42 UTC (permalink / raw)
  To: linuxtestproject.agent; +Cc: ltp

> Hi Petr,

> --- [PATCH 1/2] ---

> On 2026-04-17, Petr Vorel wrote:
> > safe_stdio: More checks for invalid function return values

> > -	if (ret == EOF)
> > -		tst_brkm_(file, lineno, TBROK | TERRNO, NULL, "fflush(%p)", f);
> > +	if (ret == EOF) {
> > +		tst_brkm_(file, lineno, TBROK | TERRNO, NULL, "fflush(%p) failed", f);
> > +	} else if (!ret) {
> > +		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
> > +				  "Invalid fflush(%p) return value %d", f, ret);
> > +	}

> !ret is ret == 0, which is fflush()'s success return — this breaks every
> caller on success. fflush() only returns 0 or EOF per POSIX; there is no
> invalid return value to guard against, so drop the else if branch entirely.

Yeah, this one is really invalid, thanks agent :).
I'll wait little longer for others feedback before sending v2.

> > +	} else if (ret < 0) {
> > +		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
> > +				  "Invalid ftell(%p) return value %ld", f, ret);

> > +	} else if (ret < 0) {
> > +		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
> > +				  "Invalid fileno(%p) return value %d", f, ret);

> Use ret < -1 here to be consistent with safe_fseek() in the same file and
> with the policy established by patch 2/2; also fix the continuation-line
> indentation (tabs only, no mixed spaces).

I frankly expect second patch being refused. If not, I'd swap commits in v2.

> --- [PATCH 2/2] ---

> On 2026-04-17, Petr Vorel wrote:
> > lib: Be pedantic on invalid comparison check

> > diff --git a/lib/safe_stdio.c b/lib/safe_stdio.c
> > @@ -67,7 +67,7 @@ int safe_asprintf(...)
> > -	} else if (ret < 0) {
> > +	} else if (ret < -1) {
> > ...
> > @@ -154,7 +154,7 @@ int safe_fseek(...)
> > -	} else if (ret < 0) {
> > +	} else if (ret < -1) {

> This fixes asprintf and fseek but misses safe_ftell() and safe_fileno()
> from patch 1/2, which are left with ret < 0. Either fix them here or
> swap the patch order so new checks are written with ret < -1 from the start.

Good catch. I was sure I would not forget, but obviously I did.

Kind regards,
Petr

> ---
> Note:

> Our agent completed the review of the patch. The full review can be found
> in the conversation context above.

> The agent can sometimes produce false positives although often its findings
> are genuine. If you find issues with the review, please comment this email
> or ignore the suggestions.

> Regards,
> LTP AI Reviewer

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-04-17  9:42 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-17  8:47 [LTP] [PATCH 1/2] safe_stdio: More checks for invalid function return values Petr Vorel
2026-04-17  8:47 ` [LTP] [PATCH 2/2] lib: Be pedantic on invalid comparison check Petr Vorel
2026-04-17  9:24 ` [LTP] safe_stdio: More checks for invalid function return values linuxtestproject.agent
2026-04-17  9:42   ` Petr Vorel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox