* [LTP] [PATCH 01/19] Unify error handling in lib/tst_safe_macros.c
2020-10-26 16:47 [LTP] [PATCH 00/19] Unify error handling in LTP library Martin Doucha
@ 2020-10-26 16:47 ` Martin Doucha
2020-10-27 8:22 ` Yang Xu
2020-10-29 15:35 ` Cyril Hrubis
2020-10-26 16:47 ` [LTP] [PATCH 02/19] Unify error handling in lib/tst_safe_sysv_ipc.c Martin Doucha
` (18 subsequent siblings)
19 siblings, 2 replies; 50+ messages in thread
From: Martin Doucha @ 2020-10-26 16:47 UTC (permalink / raw)
To: ltp
- Properly format caller file:line location
- Pedantically check for invalid syscall return values, don't ignore silently
- Always return syscall result so that all SAFE_*() functions can be called
in test cleanup
Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
include/tst_safe_macros.h | 30 ++++----
lib/tst_safe_macros.c | 154 ++++++++++++++++++++++++++++++--------
2 files changed, 137 insertions(+), 47 deletions(-)
diff --git a/include/tst_safe_macros.h b/include/tst_safe_macros.h
index fd0fcd278..42d923370 100644
--- a/include/tst_safe_macros.h
+++ b/include/tst_safe_macros.h
@@ -130,7 +130,7 @@ int safe_setreuid(const char *file, const int lineno,
int safe_setpgid(const char *file, const int lineno, pid_t pid, pid_t pgid);
#define SAFE_SETPGID(pid, pgid) \
- safe_setpgid(__FILE__, __LINE__, (pid), (pgid));
+ safe_setpgid(__FILE__, __LINE__, (pid), (pgid))
pid_t safe_getpgid(const char *file, const int lineno, pid_t pid);
@@ -141,7 +141,7 @@ pid_t safe_getpgid(const char *file, const int lineno, pid_t pid);
safe_unlink(__FILE__, __LINE__, NULL, (pathname))
#define SAFE_LINK(oldpath, newpath) \
- safe_link(__FILE__, __LINE__, NULL, (oldpath), (newpath))
+ safe_link(__FILE__, __LINE__, NULL, (oldpath), (newpath))
#define SAFE_LINKAT(olddirfd, oldpath, newdirfd, newpath, flags) \
safe_linkat(__FILE__, __LINE__, NULL, (olddirfd), (oldpath), \
@@ -151,7 +151,7 @@ pid_t safe_getpgid(const char *file, const int lineno, pid_t pid);
safe_readlink(__FILE__, __LINE__, NULL, (path), (buf), (bufsize))
#define SAFE_SYMLINK(oldpath, newpath) \
- safe_symlink(__FILE__, __LINE__, NULL, (oldpath), (newpath))
+ safe_symlink(__FILE__, __LINE__, NULL, (oldpath), (newpath))
#define SAFE_WRITE(len_strict, fildes, buf, nbyte) \
safe_write(__FILE__, __LINE__, NULL, (len_strict), (fildes), (buf), (nbyte))
@@ -178,10 +178,10 @@ pid_t safe_getpgid(const char *file, const int lineno, pid_t pid);
safe_fchown(__FILE__, __LINE__, NULL, (fd), (owner), (group))
#define SAFE_WAIT(status) \
- safe_wait(__FILE__, __LINE__, NULL, (status))
+ safe_wait(__FILE__, __LINE__, NULL, (status))
#define SAFE_WAITPID(pid, status, opts) \
- safe_waitpid(__FILE__, __LINE__, NULL, (pid), (status), (opts))
+ safe_waitpid(__FILE__, __LINE__, NULL, (pid), (status), (opts))
#define SAFE_KILL(pid, sig) \
safe_kill(__FILE__, __LINE__, NULL, (pid), (sig))
@@ -350,7 +350,7 @@ static inline int safe_statfs(const char *file, const int lineno,
return rval;
}
#define SAFE_STATFS(path, buf) \
- safe_statfs(__FILE__, __LINE__, (path), (buf))
+ safe_statfs(__FILE__, __LINE__, (path), (buf))
static inline off_t safe_lseek(const char *file, const int lineno,
int fd, off_t offset, int whence)
@@ -432,32 +432,32 @@ int safe_sigaction(const char *file, const int lineno,
#define SAFE_SIGACTION(signum, act, oldact) \
safe_sigaction(__FILE__, __LINE__, (signum), (act), (oldact))
-void safe_sigaddset(const char *file, const int lineno,
+int safe_sigaddset(const char *file, const int lineno,
sigset_t *sigs, int signo);
#define SAFE_SIGADDSET(sigs, signo) \
safe_sigaddset(__FILE__, __LINE__, (sigs), (signo))
-void safe_sigdelset(const char *file, const int lineno,
+int safe_sigdelset(const char *file, const int lineno,
sigset_t *sigs, int signo);
#define SAFE_SIGDELSET(sigs, signo) \
safe_sigdelset(__FILE__, __LINE__, (sigs), (signo))
-void safe_sigemptyset(const char *file, const int lineno,
+int safe_sigemptyset(const char *file, const int lineno,
sigset_t *sigs);
#define SAFE_SIGEMPTYSET(sigs) \
safe_sigemptyset(__FILE__, __LINE__, (sigs))
-void safe_sigfillset(const char *file, const int lineno,
+int safe_sigfillset(const char *file, const int lineno,
sigset_t *sigs);
#define SAFE_SIGFILLSET(sigs) \
safe_sigfillset(__FILE__, __LINE__, (sigs))
-void safe_sigprocmask(const char *file, const int lineno,
+int safe_sigprocmask(const char *file, const int lineno,
int how, sigset_t *set, sigset_t *oldset);
#define SAFE_SIGPROCMASK(how, set, oldset) \
safe_sigprocmask(__FILE__, __LINE__, (how), (set), (oldset))
-void safe_sigwait(const char *file, const int lineno,
+int safe_sigwait(const char *file, const int lineno,
sigset_t *set, int *sig);
#define SAFE_SIGWAIT(set, sig) \
safe_sigwait(__FILE__, __LINE__, (set), (sig))
@@ -563,11 +563,11 @@ int safe_personality(const char *filename, unsigned int lineno,
} \
} while (0)
-void safe_unshare(const char *file, const int lineno, int flags);
+int safe_unshare(const char *file, const int lineno, int flags);
#define SAFE_UNSHARE(flags) safe_unshare(__FILE__, __LINE__, (flags))
-void safe_setns(const char *file, const int lineno, int fd, int nstype);
-#define SAFE_SETNS(fd, nstype) safe_setns(__FILE__, __LINE__, (fd), (nstype));
+int safe_setns(const char *file, const int lineno, int fd, int nstype);
+#define SAFE_SETNS(fd, nstype) safe_setns(__FILE__, __LINE__, (fd), (nstype))
static inline void safe_cmd(const char *file, const int lineno, const char *const argv[],
const char *stdout_path, const char *stderr_path)
diff --git a/lib/tst_safe_macros.c b/lib/tst_safe_macros.c
index 41b2141d7..dd7f604eb 100644
--- a/lib/tst_safe_macros.c
+++ b/lib/tst_safe_macros.c
@@ -23,10 +23,14 @@ int safe_setpgid(const char *file, const int lineno, pid_t pid, pid_t pgid)
int rval;
rval = setpgid(pid, pgid);
- if (rval) {
- tst_brk(TBROK | TERRNO,
- "%s:%d: setpgid(%i, %i) failed",
- file, lineno, pid, pgid);
+
+ if (rval == -1) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "setpgid(%i, %i) failed", pid, pgid);
+ } else if (rval) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "Invalid setpgid(%i, %i) return value %d", pid, pgid,
+ rval);
}
return rval;
@@ -37,9 +41,13 @@ pid_t safe_getpgid(const char *file, const int lineno, pid_t pid)
pid_t pgid;
pgid = getpgid(pid);
+
if (pgid == -1) {
- tst_brk(TBROK | TERRNO,
- "%s:%d: getpgid(%i) failed", file, lineno, pid);
+ tst_brk_(file, lineno, TBROK | TERRNO, "getpgid(%i) failed",
+ pid);
+ } else if (pgid < 0) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "Invalid getpgid(%i) return value %d", pid, pgid);
}
return pgid;
@@ -50,9 +58,13 @@ int safe_personality(const char *filename, unsigned int lineno,
{
int prev_persona = personality(persona);
- if (prev_persona < 0) {
+ if (prev_persona == -1) {
tst_brk_(filename, lineno, TBROK | TERRNO,
"persona(%ld) failed", persona);
+ } else if (prev_persona < 0) {
+ tst_brk_(filename, lineno, TBROK | TERRNO,
+ "Invalid persona(%ld) return value %d", persona,
+ prev_persona);
}
return prev_persona;
@@ -64,10 +76,14 @@ int safe_setregid(const char *file, const int lineno,
int rval;
rval = setregid(rgid, egid);
+
if (rval == -1) {
tst_brk_(file, lineno, TBROK | TERRNO,
- "setregid(%li, %li) failed",
- (long)rgid, (long)egid);
+ "setregid(%li, %li) failed", (long)rgid, (long)egid);
+ } else if (rval) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "Invalid setregid(%li, %li) return value %d",
+ (long)rgid, (long)egid, rval);
}
return rval;
@@ -79,10 +95,14 @@ int safe_setreuid(const char *file, const int lineno,
int rval;
rval = setreuid(ruid, euid);
+
if (rval == -1) {
tst_brk_(file, lineno, TBROK | TERRNO,
- "setreuid(%li, %li) failed",
- (long)ruid, (long)euid);
+ "setreuid(%li, %li) failed", (long)ruid, (long)euid);
+ } else if (rval) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "Invalid setreuid(%li, %li) return value %d",
+ (long)ruid, (long)euid, rval);
}
return rval;
@@ -101,55 +121,87 @@ int safe_sigaction(const char *file, const int lineno,
tst_brk_(file, lineno, TBROK | TERRNO,
"sigaction(%s (%d), %p, %p) failed",
tst_strsig(signum), signum, act, oldact);
+ } else if (rval) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "Invalid sigaction(%s (%d), %p, %p) return value %d",
+ tst_strsig(signum), signum, act, oldact, rval);
}
return rval;
}
-void safe_sigaddset(const char *file, const int lineno,
+int safe_sigaddset(const char *file, const int lineno,
sigset_t *sigs, int signo)
{
int rval;
rval = sigaddset(sigs, signo);
+
if (rval == -1) {
tst_brk_(file, lineno, TBROK | TERRNO,
- "sigaddset() %s (%i) failed",
- tst_strsig(signo), signo);
+ "sigaddset() %s (%i) failed", tst_strsig(signo),
+ signo);
+ } else if (rval) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "Invalid sigaddset() %s (%i) return value %d",
+ tst_strsig(signo), signo, rval);
}
+
+ return rval;
}
-void safe_sigdelset(const char *file, const int lineno,
+int safe_sigdelset(const char *file, const int lineno,
sigset_t *sigs, int signo)
{
int rval;
rval = sigdelset(sigs, signo);
+
if (rval == -1) {
tst_brk_(file, lineno, TBROK | TERRNO,
- "sigdelset() %s (%i) failed",
- tst_strsig(signo), signo);
+ "sigdelset() %s (%i) failed", tst_strsig(signo),
+ signo);
+ } else if (rval) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "Invalid sigdelset() %s (%i) return value %d",
+ tst_strsig(signo), signo, rval);
}
+
+ return rval;
}
-void safe_sigemptyset(const char *file, const int lineno,
+int safe_sigemptyset(const char *file, const int lineno,
sigset_t *sigs)
{
int rval;
rval = sigemptyset(sigs);
- if (rval == -1)
+
+ if (rval == -1) {
tst_brk_(file, lineno, TBROK | TERRNO, "sigemptyset() failed");
+ } else if (rval) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "Invalid sigemptyset() return value %d", rval);
+ }
+
+ return rval;
}
-void safe_sigfillset(const char *file, const int lineno,
+int safe_sigfillset(const char *file, const int lineno,
sigset_t *sigs)
{
int rval;
rval = sigfillset(sigs);
- if (rval == -1)
+
+ if (rval == -1) {
tst_brk_(file, lineno, TBROK | TERRNO, "sigfillset() failed");
+ } else if (rval) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "Invalid sigfillset() return value %d", rval);
+ }
+
+ return rval;
}
static const char *strhow(int how)
@@ -166,28 +218,44 @@ static const char *strhow(int how)
}
}
-void safe_sigprocmask(const char *file, const int lineno,
+int safe_sigprocmask(const char *file, const int lineno,
int how, sigset_t *set, sigset_t *oldset)
{
int rval;
rval = sigprocmask(how, set, oldset);
+
if (rval == -1) {
tst_brk_(file, lineno, TBROK | TERRNO,
- "sigprocmask(%s, %p, %p)", strhow(how), set, oldset);
+ "sigprocmask(%s, %p, %p) failed", strhow(how), set,
+ oldset);
+ } else if (rval) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "Invalid sigprocmask(%s, %p, %p) return value %d",
+ strhow(how), set, oldset, rval);
}
+
+ return rval;
}
-void safe_sigwait(const char *file, const int lineno,
+int safe_sigwait(const char *file, const int lineno,
sigset_t *set, int *sig)
{
int rval;
rval = sigwait(set, sig);
- if (rval != 0) {
+
+ if (rval > 0) {
errno = rval;
- tst_brk_(file, lineno, TBROK, "sigwait(%p, %p)", set, sig);
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "sigwait(%p, %p) failed", set, sig);
+ } else if (rval) {
+ tst_brk_(file, lineno, TBROK,
+ "Invalid sigwait(%p, %p) return value %d", set, sig,
+ rval);
}
+
+ return rval;
}
struct group *safe_getgrnam(const char *file, const int lineno,
@@ -241,19 +309,24 @@ int safe_chroot(const char *file, const int lineno, const char *path)
int rval;
rval = chroot(path);
+
if (rval == -1) {
+ tst_brk_(file, lineno, TBROK | TERRNO, "chroot(%s) failed",
+ path);
+ } else if (rval) {
tst_brk_(file, lineno, TBROK | TERRNO,
- "chroot(%s) failed", path);
+ "Invalid chroot(%s) return value %d", path, rval);
}
return rval;
}
-void safe_unshare(const char *file, const int lineno, int flags)
+int safe_unshare(const char *file, const int lineno, int flags)
{
int res;
res = unshare(flags);
+
if (res == -1) {
if (errno == EINVAL) {
tst_brk_(file, lineno, TCONF | TERRNO,
@@ -262,18 +335,30 @@ void safe_unshare(const char *file, const int lineno, int flags)
tst_brk_(file, lineno, TBROK | TERRNO,
"unshare(%d) failed", flags);
}
+ } else if (res) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "Invalid unshare(%d) return value %d", flags, res);
}
+
+ return res;
}
-void safe_setns(const char *file, const int lineno, int fd, int nstype)
+int safe_setns(const char *file, const int lineno, int fd, int nstype)
{
int ret;
ret = setns(fd, nstype);
+
if (ret == -1) {
tst_brk_(file, lineno, TBROK | TERRNO, "setns(%i, %i) failed",
- fd, nstype);
+ fd, nstype);
+ } else if (ret) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "Invalid setns(%i, %i) return value %d", fd, nstype,
+ ret);
}
+
+ return ret;
}
long tst_safe_ptrace(const char *file, const int lineno, int req, pid_t pid,
@@ -299,10 +384,15 @@ int safe_pipe2(const char *file, const int lineno, int fildes[2], int flags)
int ret;
ret = pipe2(fildes, flags);
+
if (ret == -1) {
tst_brk_(file, lineno, TBROK | TERRNO,
- "pipe2({%d,%d}) failed with flag(%d)",
- fildes[0], fildes[1], flags);
+ "pipe2({%d,%d}) failed with flag(%d)", fildes[0],
+ fildes[1], flags);
+ } else if (ret) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "Invalid pipe2({%d,%d}, %d) return value %d",
+ fildes[0], fildes[1], flags, ret);
}
return ret;
--
2.28.0
^ permalink raw reply related [flat|nested] 50+ messages in thread* [LTP] [PATCH 02/19] Unify error handling in lib/tst_safe_sysv_ipc.c
2020-10-26 16:47 [LTP] [PATCH 00/19] Unify error handling in LTP library Martin Doucha
2020-10-26 16:47 ` [LTP] [PATCH 01/19] Unify error handling in lib/tst_safe_macros.c Martin Doucha
@ 2020-10-26 16:47 ` Martin Doucha
2020-10-27 9:10 ` Yang Xu
2020-10-26 16:47 ` [LTP] [PATCH 03/19] Unify error handling in lib/tst_safe_timerfd.c Martin Doucha
` (17 subsequent siblings)
19 siblings, 1 reply; 50+ messages in thread
From: Martin Doucha @ 2020-10-26 16:47 UTC (permalink / raw)
To: ltp
- 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>
---
lib/tst_safe_sysv_ipc.c | 79 +++++++++++++++++++++++++++++------------
1 file changed, 56 insertions(+), 23 deletions(-)
diff --git a/lib/tst_safe_sysv_ipc.c b/lib/tst_safe_sysv_ipc.c
index 30b5f6ec7..49c929b2a 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,15 @@ int safe_shmctl(const char *file, const int lineno, int shmid, int cmd,
int rval;
rval = shmctl(shmid, cmd, buf);
+
+ if (rval == -1) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "shmctl(%i, %i, %p) failed", 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);
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "Invalid shmctl(%i, %i, %p) return value %d", shmid,
+ cmd, buf, rval);
}
return rval;
--
2.28.0
^ permalink raw reply related [flat|nested] 50+ messages in thread* [LTP] [PATCH 02/19] Unify error handling in lib/tst_safe_sysv_ipc.c
2020-10-26 16:47 ` [LTP] [PATCH 02/19] Unify error handling in lib/tst_safe_sysv_ipc.c Martin Doucha
@ 2020-10-27 9:10 ` Yang Xu
0 siblings, 0 replies; 50+ messages in thread
From: Yang Xu @ 2020-10-27 9:10 UTC (permalink / raw)
To: ltp
Hi Martin
> @@ -135,10 +163,15 @@ int safe_shmctl(const char *file, const int lineno, int shmid, int cmd,
> int rval;
>
> rval = shmctl(shmid, cmd, buf);
> +
> + if (rval == -1) {
> + tst_brk_(file, lineno, TBROK | TERRNO,
> + "shmctl(%i, %i, %p) failed", shmid, cmd, buf);
> + }
> if (ret_check(cmd, rval)) {
Here should use else, otherwise it will print two lines error because
tst_brk_ will not exit in cleanup.
Best Regards
Yang Xu
> - tst_brk(TBROK | TERRNO,
> - "%s:%d: shmctl(%i, %i, %p) = %i failed",
> - file, lineno, shmid, cmd, buf, rval);
> + tst_brk_(file, lineno, TBROK | TERRNO,
> + "Invalid shmctl(%i, %i, %p) return value %d", shmid,
> + cmd, buf, rval);
> }
>
> return rval;
^ permalink raw reply [flat|nested] 50+ messages in thread
* [LTP] [PATCH 03/19] Unify error handling in lib/tst_safe_timerfd.c
2020-10-26 16:47 [LTP] [PATCH 00/19] Unify error handling in LTP library Martin Doucha
2020-10-26 16:47 ` [LTP] [PATCH 01/19] Unify error handling in lib/tst_safe_macros.c Martin Doucha
2020-10-26 16:47 ` [LTP] [PATCH 02/19] Unify error handling in lib/tst_safe_sysv_ipc.c Martin Doucha
@ 2020-10-26 16:47 ` Martin Doucha
2020-10-27 9:21 ` Yang Xu
2020-10-26 16:47 ` [LTP] [PATCH 04/19] Unify error handling in lib/safe_file_ops.c Martin Doucha
` (16 subsequent siblings)
19 siblings, 1 reply; 50+ messages in thread
From: Martin Doucha @ 2020-10-26 16:47 UTC (permalink / raw)
To: ltp
- Properly format caller file:line locations
- Pedantically check for invalid syscall return values
Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
lib/tst_safe_timerfd.c | 32 +++++++++++++++++++++++---------
1 file changed, 23 insertions(+), 9 deletions(-)
diff --git a/lib/tst_safe_timerfd.c b/lib/tst_safe_timerfd.c
index ffe7b2ef7..4c36a309c 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,14 @@ 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");
+ }
+ if (rval) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "Invalid timerfd_gettime() return value %d", rval);
}
return rval;
@@ -47,9 +57,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
^ permalink raw reply related [flat|nested] 50+ messages in thread* [LTP] [PATCH 03/19] Unify error handling in lib/tst_safe_timerfd.c
2020-10-26 16:47 ` [LTP] [PATCH 03/19] Unify error handling in lib/tst_safe_timerfd.c Martin Doucha
@ 2020-10-27 9:21 ` Yang Xu
2020-11-06 17:35 ` Petr Vorel
0 siblings, 1 reply; 50+ messages in thread
From: Yang Xu @ 2020-10-27 9:21 UTC (permalink / raw)
To: ltp
Hi Martin
> - Properly format caller file:line locations
> - Pedantically check for invalid syscall return values
>
> Signed-off-by: Martin Doucha<mdoucha@suse.cz>
> ---
> lib/tst_safe_timerfd.c | 32 +++++++++++++++++++++++---------
> 1 file changed, 23 insertions(+), 9 deletions(-)
>
> diff --git a/lib/tst_safe_timerfd.c b/lib/tst_safe_timerfd.c
> index ffe7b2ef7..4c36a309c 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,14 @@ 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");
> + }
> + if (rval) {
> + tst_brk_(file, lineno, TBROK | TERRNO,
> + "Invalid timerfd_gettime() return value %d", rval);
Here also should use else if.
> }
>
> return rval;
> @@ -47,9 +57,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;
^ permalink raw reply [flat|nested] 50+ messages in thread* [LTP] [PATCH 03/19] Unify error handling in lib/tst_safe_timerfd.c
2020-10-27 9:21 ` Yang Xu
@ 2020-11-06 17:35 ` Petr Vorel
2020-11-07 0:22 ` Yang Xu
0 siblings, 1 reply; 50+ messages in thread
From: Petr Vorel @ 2020-11-06 17:35 UTC (permalink / raw)
To: ltp
Hi Martin, Xu,
> > diff --git a/lib/tst_safe_timerfd.c b/lib/tst_safe_timerfd.c
> > index ffe7b2ef7..4c36a309c 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) {
nit: fd< 0 => 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,14 @@ 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");
> > + }
> > + if (rval) {
> > + tst_brk_(file, lineno, TBROK | TERRNO,
> > + "Invalid timerfd_gettime() return value %d", rval);
> Here also should use else if.
+1.
If I'm not wrong we need to count that safe functions does not quit the test in
cleanup function (tst_brk__() and tst_brk_entered).
Kind regards,
Petr
^ permalink raw reply [flat|nested] 50+ messages in thread* [LTP] [PATCH 03/19] Unify error handling in lib/tst_safe_timerfd.c
2020-11-06 17:35 ` Petr Vorel
@ 2020-11-07 0:22 ` Yang Xu
2020-11-07 16:42 ` Petr Vorel
0 siblings, 1 reply; 50+ messages in thread
From: Yang Xu @ 2020-11-07 0:22 UTC (permalink / raw)
To: ltp
Hi Petr, Martin
> Hi Martin, Xu,
>
>>> diff --git a/lib/tst_safe_timerfd.c b/lib/tst_safe_timerfd.c
>>> index ffe7b2ef7..4c36a309c 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) {
> nit: fd< 0 => fd< 0
Sorry, It is my email format problem, on patchwork[1], the format is right.
[1]https://patchwork.ozlabs.org/project/ltp/patch/20201026164756.30556-4-mdoucha@suse.cz/
>>> + tst_brk_(file, lineno, TBROK | TERRNO,
>>> + "Invalid timerfd_create(%s) return value %d",
>>> + tst_clock_name(clockid), fd);
>
>>> }
>
>>> return fd;
>>> @@ -31,9 +36,14 @@ 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");
>>> + }
>>> + if (rval) {
>>> + tst_brk_(file, lineno, TBROK | TERRNO,
>>> + "Invalid timerfd_gettime() return value %d", rval);
>> Here also should use else if.
> +1.
> If I'm not wrong we need to count that safe functions does not quit the test in
> cleanup function (tst_brk__() and tst_brk_entered).
Yes, because we want to do more it in cleanup, so we use
"tst_brk_handler = tst_cvres" to print something instead of "
tst_brk_handler = tst_vbrk_;". We also have documented it in
doc/test-writing-guidelines.txt.
Best Regards
Yang Xu
>
> Kind regards,
> Petr
>
>
> .
>
^ permalink raw reply [flat|nested] 50+ messages in thread* [LTP] [PATCH 03/19] Unify error handling in lib/tst_safe_timerfd.c
2020-11-07 0:22 ` Yang Xu
@ 2020-11-07 16:42 ` Petr Vorel
0 siblings, 0 replies; 50+ messages in thread
From: Petr Vorel @ 2020-11-07 16:42 UTC (permalink / raw)
To: ltp
Hi Martin, Xu,
> > Hi Martin, Xu,
> > > > diff --git a/lib/tst_safe_timerfd.c b/lib/tst_safe_timerfd.c
> > > > index ffe7b2ef7..4c36a309c 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) {
> > nit: fd< 0 => fd< 0
> Sorry, It is my email format problem, on patchwork[1], the format is right.
Martin, sorry for not checking what you send in the patch.
> [1]https://patchwork.ozlabs.org/project/ltp/patch/20201026164756.30556-4-mdoucha@suse.cz/
Kind regards,
Petr
^ permalink raw reply [flat|nested] 50+ messages in thread
* [LTP] [PATCH 04/19] Unify error handling in lib/safe_file_ops.c
2020-10-26 16:47 [LTP] [PATCH 00/19] Unify error handling in LTP library Martin Doucha
` (2 preceding siblings ...)
2020-10-26 16:47 ` [LTP] [PATCH 03/19] Unify error handling in lib/tst_safe_timerfd.c Martin Doucha
@ 2020-10-26 16:47 ` Martin Doucha
2020-10-29 15:59 ` Cyril Hrubis
2020-10-26 16:47 ` [LTP] [PATCH 05/19] Unify error handling in lib/safe_macros.c Martin Doucha
` (15 subsequent siblings)
19 siblings, 1 reply; 50+ messages in thread
From: Martin Doucha @ 2020-10-26 16:47 UTC (permalink / raw)
To: ltp
- 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>
---
include/safe_file_ops_fn.h | 8 +-
lib/safe_file_ops.c | 207 +++++++++++++++++++++----------------
2 files changed, 120 insertions(+), 95 deletions(-)
diff --git a/include/safe_file_ops_fn.h b/include/safe_file_ops_fn.h
index 052fb1b9a..98730de82 100644
--- a/include/safe_file_ops_fn.h
+++ b/include/safe_file_ops_fn.h
@@ -30,7 +30,7 @@ int file_scanf(const char *file, const int lineno,
const char *path, const char *fmt, ...)
__attribute__ ((format (scanf, 4, 5)));
-void safe_file_scanf(const char *file, const int lineno,
+int safe_file_scanf(const char *file, const int lineno,
void (*cleanup_fn)(void),
const char *path, const char *fmt, ...)
__attribute__ ((format (scanf, 5, 6)));
@@ -47,7 +47,7 @@ int file_printf(const char *file, const int lineno,
const char *path, const char *fmt, ...)
__attribute__ ((format (printf, 4, 5)));
-void safe_file_printf(const char *file, const int lineno,
+int safe_file_printf(const char *file, const int lineno,
void (*cleanup_fn)(void),
const char *path, const char *fmt, ...)
__attribute__ ((format (printf, 5, 6)));
@@ -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..a63368875 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,14 +118,14 @@ 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;
}
-void safe_file_scanf(const char *file, const int lineno,
+int safe_file_scanf(const char *file, const int lineno,
void (*cleanup_fn) (void),
const char *path, const char *fmt, ...)
{
@@ -139,10 +136,9 @@ 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);
- return;
+ tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+ "Failed to open FILE '%s' for reading", path);
+ return 1;
}
exp_convs = count_scanf_conversions(fmt);
@@ -152,25 +148,25 @@ 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);
- return;
+ tst_brkm_(file, lineno, TBROK, cleanup_fn,
+ "The FILE '%s' ended prematurely", path);
+ return 1;
}
if (ret != exp_convs) {
- tst_brkm(TBROK, cleanup_fn,
- "Expected %i conversions got %i FILE '%s' at %s:%d",
- exp_convs, ret, path, file, lineno);
- return;
+ tst_brkm_(file, lineno, TBROK, cleanup_fn,
+ "Expected %i conversions got %i FILE '%s'",
+ exp_convs, ret, path);
+ return 1;
}
if (fclose(f)) {
- tst_brkm(TBROK | TERRNO, cleanup_fn,
- "Failed to close FILE '%s' at %s:%d",
- path, file, lineno);
- return;
+ tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+ "Failed to close FILE '%s'", path);
+ return 1;
}
+
+ return 0;
}
@@ -190,16 +186,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 +210,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 +228,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,14 +253,14 @@ 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;
}
-void safe_file_printf(const char *file, const int lineno,
+int safe_file_printf(const char *file, const int lineno,
void (*cleanup_fn) (void),
const char *path, const char *fmt, ...)
{
@@ -278,33 +270,32 @@ 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);
- return;
+ tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+ "Failed to open FILE '%s' for writing", path);
+ return 1;
}
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);
- return;
+ tst_brkm_(file, lineno, TBROK, cleanup_fn,
+ "Failed to print to FILE '%s'", path);
+ return 1;
}
va_end(va);
if (fclose(f)) {
- tst_brkm(TBROK | TERRNO, cleanup_fn,
- "Failed to close FILE '%s' at %s:%d",
- path, file, lineno);
- return;
+ tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+ "Failed to close FILE '%s'", path);
+ return 1;
}
+
+ return 0;
}
//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 +307,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 +335,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 +346,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 +395,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 +430,18 @@ void safe_touch(const char *file, const int lineno,
}
#endif
if (ret == -1) {
- tst_brkm(TBROK | TERRNO, cleanup_fn,
+ tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
"Failed to update the access/modification time on file"
- " '%s'@%s:%d", pathname, file, lineno);
+ " '%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
^ permalink raw reply related [flat|nested] 50+ messages in thread* [LTP] [PATCH 04/19] Unify error handling in lib/safe_file_ops.c
2020-10-26 16:47 ` [LTP] [PATCH 04/19] Unify error handling in lib/safe_file_ops.c Martin Doucha
@ 2020-10-29 15:59 ` Cyril Hrubis
2020-10-29 16:02 ` Cyril Hrubis
0 siblings, 1 reply; 50+ messages in thread
From: Cyril Hrubis @ 2020-10-29 15:59 UTC (permalink / raw)
To: ltp
Hi!
> diff --git a/lib/safe_file_ops.c b/lib/safe_file_ops.c
> index e06d399fa..a63368875 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;
scanf() returns negative value on error, I guess it would make more
sense to return -1 here and in many cases below.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 50+ messages in thread* [LTP] [PATCH 04/19] Unify error handling in lib/safe_file_ops.c
2020-10-29 15:59 ` Cyril Hrubis
@ 2020-10-29 16:02 ` Cyril Hrubis
2020-10-29 16:05 ` Martin Doucha
0 siblings, 1 reply; 50+ messages in thread
From: Cyril Hrubis @ 2020-10-29 16:02 UTC (permalink / raw)
To: ltp
Hi!
> > diff --git a/lib/safe_file_ops.c b/lib/safe_file_ops.c
> > index e06d399fa..a63368875 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;
>
> scanf() returns negative value on error, I guess it would make more
> sense to return -1 here and in many cases below.
That's true for printf() scanf returns EOF instead. But I guess that
anything < 0 would work better than 1 which means that one input item
was matched successfuly...
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 50+ messages in thread* [LTP] [PATCH 04/19] Unify error handling in lib/safe_file_ops.c
2020-10-29 16:02 ` Cyril Hrubis
@ 2020-10-29 16:05 ` Martin Doucha
2020-10-29 16:17 ` Cyril Hrubis
0 siblings, 1 reply; 50+ messages in thread
From: Martin Doucha @ 2020-10-29 16:05 UTC (permalink / raw)
To: ltp
On 29. 10. 20 17:02, Cyril Hrubis wrote:
> Hi!
>>> diff --git a/lib/safe_file_ops.c b/lib/safe_file_ops.c
>>> index e06d399fa..a63368875 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;
>>
>> scanf() returns negative value on error, I guess it would make more
>> sense to return -1 here and in many cases below.
>
> That's true for printf() scanf returns EOF instead. But I guess that
> anything < 0 would work better than 1 which means that one input item
> was matched successfuly...
These safe file functions could use some additional improvements but
changing the return value is out of scope of my patchset. That would
probably require reviewing and modifying some test code as well. The
patchset is over 4000 lines long as is.
--
Martin Doucha mdoucha@suse.cz
QA Engineer for Software Maintenance
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic
^ permalink raw reply [flat|nested] 50+ messages in thread* [LTP] [PATCH 04/19] Unify error handling in lib/safe_file_ops.c
2020-10-29 16:05 ` Martin Doucha
@ 2020-10-29 16:17 ` Cyril Hrubis
2020-10-29 16:23 ` Martin Doucha
0 siblings, 1 reply; 50+ messages in thread
From: Cyril Hrubis @ 2020-10-29 16:17 UTC (permalink / raw)
To: ltp
Hi!
> > That's true for printf() scanf returns EOF instead. But I guess that
> > anything < 0 would work better than 1 which means that one input item
> > was matched successfuly...
>
> These safe file functions could use some additional improvements but
> changing the return value is out of scope of my patchset. That would
> probably require reviewing and modifying some test code as well. The
> patchset is over 4000 lines long as is.
It's actually not since you are chaning the void functions to return
int, if you kept them void that would mean that it's out of scope.
Also actually FILE_PRINTF() can be replaced SAFE_FILE_PRINTF() for all
new library tests since tst_brk() does not exit cleanup() anymore so it
may as well be easier to just get rid of FILE_PRINTF() in new test
library.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 50+ messages in thread
* [LTP] [PATCH 04/19] Unify error handling in lib/safe_file_ops.c
2020-10-29 16:17 ` Cyril Hrubis
@ 2020-10-29 16:23 ` Martin Doucha
2020-10-30 10:31 ` Cyril Hrubis
0 siblings, 1 reply; 50+ messages in thread
From: Martin Doucha @ 2020-10-29 16:23 UTC (permalink / raw)
To: ltp
On 29. 10. 20 17:17, Cyril Hrubis wrote:
> Hi!
>>> That's true for printf() scanf returns EOF instead. But I guess that
>>> anything < 0 would work better than 1 which means that one input item
>>> was matched successfuly...
>>
>> These safe file functions could use some additional improvements but
>> changing the return value is out of scope of my patchset. That would
>> probably require reviewing and modifying some test code as well. The
>> patchset is over 4000 lines long as is.
>
> It's actually not since you are chaning the void functions to return
> int, if you kept them void that would mean that it's out of scope.
Changing return type from void to int is fully backwards compatible
because the return value is ignored everywhere anyway. On the other
hand, changing return value in functions which already return int could
break a lot of existing test code. Especially when you do it in a
control flow branch that doesn't terminates the test through tst_brk().
--
Martin Doucha mdoucha@suse.cz
QA Engineer for Software Maintenance
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic
^ permalink raw reply [flat|nested] 50+ messages in thread
* [LTP] [PATCH 04/19] Unify error handling in lib/safe_file_ops.c
2020-10-29 16:23 ` Martin Doucha
@ 2020-10-30 10:31 ` Cyril Hrubis
0 siblings, 0 replies; 50+ messages in thread
From: Cyril Hrubis @ 2020-10-30 10:31 UTC (permalink / raw)
To: ltp
Hi!
> >> These safe file functions could use some additional improvements but
> >> changing the return value is out of scope of my patchset. That would
> >> probably require reviewing and modifying some test code as well. The
> >> patchset is over 4000 lines long as is.
> >
> > It's actually not since you are chaning the void functions to return
> > int, if you kept them void that would mean that it's out of scope.
>
> Changing return type from void to int is fully backwards compatible
> because the return value is ignored everywhere anyway.
However it exposes suboptimal interface that shouldn't have been exposed
in the first place. So I would rather leave these to return void so that
no new code starts to depend on the current behavior.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 50+ messages in thread
* [LTP] [PATCH 05/19] Unify error handling in lib/safe_macros.c
2020-10-26 16:47 [LTP] [PATCH 00/19] Unify error handling in LTP library Martin Doucha
` (3 preceding siblings ...)
2020-10-26 16:47 ` [LTP] [PATCH 04/19] Unify error handling in lib/safe_file_ops.c Martin Doucha
@ 2020-10-26 16:47 ` Martin Doucha
2020-10-27 10:10 ` Yang Xu
2020-11-11 12:58 ` Cyril Hrubis
2020-10-26 16:47 ` [LTP] [PATCH 06/19] Unify error handling in lib/safe_net.c Martin Doucha
` (14 subsequent siblings)
19 siblings, 2 replies; 50+ messages in thread
From: Martin Doucha @ 2020-10-26 16:47 UTC (permalink / raw)
To: ltp
- Properly format caller file:line location
- Pedantically check invalid syscall return values
Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
lib/safe_macros.c | 602 +++++++++++++++++++++++++++++-----------------
1 file changed, 384 insertions(+), 218 deletions(-)
diff --git a/lib/safe_macros.c b/lib/safe_macros.c
index 4f48d7529..f5e80fc48 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,16 @@ 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);
+ }
+ 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 +309,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 +328,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 +347,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 +366,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 +385,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 +404,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 +423,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 +445,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 +465,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 +486,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 +513,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 +530,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 +554,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 +587,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 +612,19 @@ 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 +639,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 +658,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,0%o) failed", fd, mode);
+ } else if (rval) {
+ tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+ "Invalid fchmod(%d,0%o) return value %d", fd, mode,
+ rval);
}
return rval;
@@ -590,9 +677,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 +696,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 +713,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 +731,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 +750,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 +767,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 +786,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 +805,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 +841,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 +863,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 +896,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 +914,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;
@@ -812,8 +929,8 @@ 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);
+ tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+ "closedir(%p) failed", dirp);
}
return rval;
@@ -829,8 +946,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 +960,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 +983,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");
}
- 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 +1007,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");
}
- 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 +1033,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");
}
- 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 +1059,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");
}
- 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 +1085,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");
}
- 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 +1110,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");
}
- 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 +1135,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");
}
- 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 +1159,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 +1175,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 +1190,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 +1208,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 +1226,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 +1244,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
^ permalink raw reply related [flat|nested] 50+ messages in thread* [LTP] [PATCH 05/19] Unify error handling in lib/safe_macros.c
2020-10-26 16:47 ` [LTP] [PATCH 05/19] Unify error handling in lib/safe_macros.c Martin Doucha
@ 2020-10-27 10:10 ` Yang Xu
2020-10-27 10:51 ` Martin Doucha
2020-11-11 12:58 ` Cyril Hrubis
1 sibling, 1 reply; 50+ messages in thread
From: Yang Xu @ 2020-10-27 10:10 UTC (permalink / raw)
To: ltp
Hi Martin
> - Properly format caller file:line location
> - Pedantically check invalid syscall return values
>
> Signed-off-by: Martin Doucha<mdoucha@suse.cz>
> ---
> lib/safe_macros.c | 602 +++++++++++++++++++++++++++++-----------------
> 1 file changed, 384 insertions(+), 218 deletions(-)
>
> diff --git a/lib/safe_macros.c b/lib/safe_macros.c
> index 4f48d7529..f5e80fc48 100644
> --- a/lib/safe_macros.c
> +++ b/lib/safe_macros.c
>
>
> return rval;
> @@ -255,10 +288,16 @@ 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);
> + }
> + if (rval< 0) {
> + tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
> + "Invalid read(%d,%p,%zu) return value %zd", fildes,
> + buf, nbyte, rval);
> }
Here has problem.. Maybe we can use simple
if (rval < 0 || (len_strict&& (size_t)rval != nbyte)) to replace.
>
> return rval;
> return rval;
> @@ -452,10 +530,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);
> }
I prefer to use "if (rval < 0 || (len_strict&& (size_t)rval != nbyte)"
>
> }
>
> @@ -530,20 +612,19 @@ long safe_sysconf(const char *file, const int lineno,
> void (cleanup_fn) (void), int name)
> {
> long rval;
> - errno = 0;
>
> + errno = 0;
It looks no change.
> 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);
> }
^ permalink raw reply [flat|nested] 50+ messages in thread* [LTP] [PATCH 05/19] Unify error handling in lib/safe_macros.c
2020-10-27 10:10 ` Yang Xu
@ 2020-10-27 10:51 ` Martin Doucha
0 siblings, 0 replies; 50+ messages in thread
From: Martin Doucha @ 2020-10-27 10:51 UTC (permalink / raw)
To: ltp
On 27. 10. 20 11:10, Yang Xu wrote:
> Hi Martin
>> - Properly format caller file:line location
>> - Pedantically check invalid syscall return values
>>
>> Signed-off-by: Martin Doucha<mdoucha@suse.cz>
>> ---
>> ? lib/safe_macros.c | 602 +++++++++++++++++++++++++++++-----------------
>> ? 1 file changed, 384 insertions(+), 218 deletions(-)
>>
>> diff --git a/lib/safe_macros.c b/lib/safe_macros.c
>> index 4f48d7529..f5e80fc48 100644
>> --- a/lib/safe_macros.c
>> +++ b/lib/safe_macros.c
>
>>
>>
>> ????? return rval;
>> @@ -255,10 +288,16 @@ 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);
>> +??? }
>> +??? if (rval<? 0) {
>> +??????? tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
>> +??????????? "Invalid read(%d,%p,%zu) return value %zd", fildes,
>> +??????????? buf, nbyte, rval);
>> ????? }
> Here has problem.. Maybe we can? use simple
> if (rval < 0 || (len_strict&&? (size_t)rval != nbyte)) to replace.
No, this is correct except for the missing "else" again. rval < -1 is
invalid and rval == -1 will be covered by the first if() branch.
>>
>> ????? return rval;
>
>> ????? return rval;
>> @@ -452,10 +530,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);
>> ????? }
> I prefer to use "if (rval < 0 || (len_strict&&? (size_t)rval != nbyte)"
This is correct as is. rval < -1 is invalid.
>>
>
>> ????? }
>>
>> @@ -530,20 +612,19 @@ long safe_sysconf(const char *file, const int
>> lineno,
>> ??????????? void (cleanup_fn) (void), int name)
>> ? {
>> ????? long rval;
>> -??? errno = 0;
>>
>> +??? errno = 0;
> It looks no change.
There's whitespace change because checkpatch.pl was complaining.
I'll resubmit v2 for patches 2, 3 and 5 after the rest of this patchset
gets pushed. These three patches are not blocking anything else.
--
Martin Doucha mdoucha@suse.cz
QA Engineer for Software Maintenance
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic
^ permalink raw reply [flat|nested] 50+ messages in thread
* [LTP] [PATCH 05/19] Unify error handling in lib/safe_macros.c
2020-10-26 16:47 ` [LTP] [PATCH 05/19] Unify error handling in lib/safe_macros.c Martin Doucha
2020-10-27 10:10 ` Yang Xu
@ 2020-11-11 12:58 ` Cyril Hrubis
1 sibling, 0 replies; 50+ messages in thread
From: Cyril Hrubis @ 2020-11-11 12:58 UTC (permalink / raw)
To: ltp
Hi!
> @@ -255,10 +288,16 @@ 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);
> + }
> + if (rval < 0) {
> + tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
> + "Invalid read(%d,%p,%zu) return value %zd", fildes,
> + buf, nbyte, rval);
> }
Shouldn't this be else if as well? Since otherwise we may generate two
warning messages if in cleanup() read() returns -1.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 50+ messages in thread
* [LTP] [PATCH 06/19] Unify error handling in lib/safe_net.c
2020-10-26 16:47 [LTP] [PATCH 00/19] Unify error handling in LTP library Martin Doucha
` (4 preceding siblings ...)
2020-10-26 16:47 ` [LTP] [PATCH 05/19] Unify error handling in lib/safe_macros.c Martin Doucha
@ 2020-10-26 16:47 ` Martin Doucha
2020-11-11 12:53 ` Cyril Hrubis
2020-10-26 16:47 ` [LTP] [PATCH 07/19] Unify error handling in lib/safe_stdio.c Martin Doucha
` (13 subsequent siblings)
19 siblings, 1 reply; 50+ messages in thread
From: Martin Doucha @ 2020-10-26 16:47 UTC (permalink / raw)
To: ltp
- 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>
---
lib/safe_net.c | 262 +++++++++++++++++++++++++++++--------------------
1 file changed, 156 insertions(+), 106 deletions(-)
diff --git a/lib/safe_net.c b/lib/safe_net.c
index 499368007..1c486b209 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,16 @@ 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 +192,15 @@ 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 +214,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 +236,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 +260,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 +283,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 +303,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 +348,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 +367,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 +387,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 +411,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 +431,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 +449,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 +472,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
^ permalink raw reply related [flat|nested] 50+ messages in thread* [LTP] [PATCH 06/19] Unify error handling in lib/safe_net.c
2020-10-26 16:47 ` [LTP] [PATCH 06/19] Unify error handling in lib/safe_net.c Martin Doucha
@ 2020-11-11 12:53 ` Cyril Hrubis
0 siblings, 0 replies; 50+ messages in thread
From: Cyril Hrubis @ 2020-11-11 12:53 UTC (permalink / raw)
To: ltp
Hi!
> + tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
> + "Invalid getsockopt(%d, %d, %d, %p, %p) return "
> + "value %d", sockfd, level, optname, optval, optlen,
Here as well, please do not split strings even if they end up over 80
chars.
Otherwise the chages looks fine.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 50+ messages in thread
* [LTP] [PATCH 07/19] Unify error handling in lib/safe_stdio.c
2020-10-26 16:47 [LTP] [PATCH 00/19] Unify error handling in LTP library Martin Doucha
` (5 preceding siblings ...)
2020-10-26 16:47 ` [LTP] [PATCH 06/19] Unify error handling in lib/safe_net.c Martin Doucha
@ 2020-10-26 16:47 ` Martin Doucha
2020-11-10 16:11 ` Cyril Hrubis
2020-10-26 16:47 ` [LTP] [PATCH 08/19] Unify error handling in lib/tst_mkfs.c Martin Doucha
` (12 subsequent siblings)
19 siblings, 1 reply; 50+ messages in thread
From: Martin Doucha @ 2020-10-26 16:47 UTC (permalink / raw)
To: ltp
- Properly format caller file:line location
- Pedantically check invalid function return values
Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
lib/safe_stdio.c | 34 +++++++++++++++++++---------------
1 file changed, 19 insertions(+), 15 deletions(-)
diff --git a/lib/safe_stdio.c b/lib/safe_stdio.c
index 966a039a5..ab23e43bb 100644
--- a/lib/safe_stdio.c
+++ b/lib/safe_stdio.c
@@ -29,9 +29,8 @@ FILE *safe_fopen(const char *file, const int lineno, void (cleanup_fn)(void),
FILE *f = fopen(path, mode);
if (f == NULL) {
- tst_brkm(TBROK | TERRNO, cleanup_fn,
- "%s:%d: fopen(%s,%s) failed",
- file, lineno, path, mode);
+ tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+ "fopen(%s,%s) failed", path, mode);
}
return f;
@@ -44,9 +43,12 @@ int safe_fclose(const char *file, const int lineno, void (cleanup_fn)(void),
ret = fclose(f);
- if (ret) {
- tst_brkm(TBROK | TERRNO, cleanup_fn,
- "%s:%d: fclose(%p) failed", file, lineno, f);
+ if (ret == EOF) {
+ tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+ "fclose(%p) failed", f);
+ } else if (ret) {
+ tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+ "Invalid fclose(%p) return value %d", f, ret);
}
return ret;
@@ -62,9 +64,12 @@ int safe_asprintf(const char *file, const int lineno, void (cleanup_fn)(void),
ret = vasprintf(strp, fmt, va);
va_end(va);
- if (ret < 0) {
- tst_brkm(TBROK | TERRNO, cleanup_fn,
- "%s:%d: asprintf(%s,...) failed", file, lineno, fmt);
+ if (ret == -1) {
+ tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+ "asprintf(%s,...) failed", fmt);
+ } else if (ret < 0) {
+ tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+ "Invalid asprintf(%s,...) return value %d", fmt, ret);
}
return ret;
@@ -81,13 +86,12 @@ FILE *safe_popen(const char *file, const int lineno, void (cleanup_fn)(void),
if (stream == NULL) {
if (errno != 0) {
- tst_brkm(TBROK | TERRNO, cleanup_fn,
- "%s:%d: popen(%s,%s) failed",
- file, lineno, command, type);
+ tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+ "popen(%s,%s) failed", command, type);
} else {
- tst_brkm(TBROK, cleanup_fn,
- "%s:%d: popen(%s,%s) failed: Out of memory",
- file, lineno, command, type);
+ tst_brkm_(file, lineno, TBROK, cleanup_fn,
+ "popen(%s,%s) failed: Out of memory",
+ command, type);
}
}
--
2.28.0
^ permalink raw reply related [flat|nested] 50+ messages in thread* [LTP] [PATCH 08/19] Unify error handling in lib/tst_mkfs.c
2020-10-26 16:47 [LTP] [PATCH 00/19] Unify error handling in LTP library Martin Doucha
` (6 preceding siblings ...)
2020-10-26 16:47 ` [LTP] [PATCH 07/19] Unify error handling in lib/safe_stdio.c Martin Doucha
@ 2020-10-26 16:47 ` Martin Doucha
2020-11-10 15:43 ` Cyril Hrubis
2020-10-26 16:47 ` [LTP] [PATCH 09/19] Unify error handling in lib/tst_checkpoint.c Martin Doucha
` (11 subsequent siblings)
19 siblings, 1 reply; 50+ messages in thread
From: Martin Doucha @ 2020-10-26 16:47 UTC (permalink / raw)
To: ltp
- Properly format caller file:line location
Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
lib/tst_mkfs.c | 36 +++++++++++++++++++-----------------
1 file changed, 19 insertions(+), 17 deletions(-)
diff --git a/lib/tst_mkfs.c b/lib/tst_mkfs.c
index 38b2e7151..45dd83ad3 100644
--- a/lib/tst_mkfs.c
+++ b/lib/tst_mkfs.c
@@ -33,14 +33,14 @@ void tst_mkfs_(const char *file, const int lineno, void (cleanup_fn)(void),
char extra_opts_str[1024] = "";
if (!dev) {
- tst_brkm(TBROK, cleanup_fn,
- "%s:%d: No device specified", file, lineno);
+ tst_brkm_(file, lineno, TBROK, cleanup_fn,
+ "No device specified");
return;
}
if (!fs_type) {
- tst_brkm(TBROK, cleanup_fn,
- "%s:%d: No fs_type specified", file, lineno);
+ tst_brkm_(file, lineno, TBROK, cleanup_fn,
+ "No fs_type specified");
return;
}
@@ -51,9 +51,8 @@ void tst_mkfs_(const char *file, const int lineno, void (cleanup_fn)(void),
argv[pos++] = fs_opts[i];
if (pos + 2 > OPTS_MAX) {
- tst_brkm(TBROK, cleanup_fn,
- "%s:%d: Too much mkfs options",
- file, lineno);
+ tst_brkm_(file, lineno, TBROK, cleanup_fn,
+ "Too many mkfs options");
return;
}
@@ -70,8 +69,8 @@ void tst_mkfs_(const char *file, const int lineno, void (cleanup_fn)(void),
argv[pos++] = extra_opts[i];
if (pos + 1 > OPTS_MAX) {
- tst_brkm(TBROK, cleanup_fn,
- "%s:%d: Too much mkfs options", file, lineno);
+ tst_brkm_(file, lineno, TBROK, cleanup_fn,
+ "Too many mkfs options");
return;
}
@@ -83,11 +82,14 @@ void tst_mkfs_(const char *file, const int lineno, void (cleanup_fn)(void),
argv[pos] = NULL;
- if (tst_clear_device(dev))
- tst_brkm(TBROK, cleanup_fn, "tst_clear_device() failed");
+ if (tst_clear_device(dev)) {
+ tst_brkm_(file, lineno, TBROK, cleanup_fn,
+ "tst_clear_device() failed");
+ }
- tst_resm(TINFO, "Formatting %s with %s opts='%s' extra opts='%s'",
- dev, fs_type, fs_opts_str, extra_opts_str);
+ tst_resm_(file, lineno, TINFO,
+ "Formatting %s with %s opts='%s' extra opts='%s'",
+ dev, fs_type, fs_opts_str, extra_opts_str);
ret = tst_cmd(cleanup_fn, argv, "/dev/null", NULL, TST_CMD_PASS_RETVAL |
TST_CMD_TCONF_ON_MISSING);
@@ -95,12 +97,12 @@ void tst_mkfs_(const char *file, const int lineno, void (cleanup_fn)(void),
case 0:
break;
case 255:
- tst_brkm(TCONF, cleanup_fn,
- "%s:%d: %s not found in $PATH", file, lineno, mkfs);
+ tst_brkm_(file, lineno, TCONF, cleanup_fn,
+ "%s not found in $PATH", mkfs);
break;
default:
- tst_brkm(TBROK, cleanup_fn,
- "%s:%d: %s failed with %i", file, lineno, mkfs, ret);
+ tst_brkm_(file, lineno, TBROK, cleanup_fn,
+ "%s failed with exit code %i", mkfs, ret);
}
}
--
2.28.0
^ permalink raw reply related [flat|nested] 50+ messages in thread* [LTP] [PATCH 09/19] Unify error handling in lib/tst_checkpoint.c
2020-10-26 16:47 [LTP] [PATCH 00/19] Unify error handling in LTP library Martin Doucha
` (7 preceding siblings ...)
2020-10-26 16:47 ` [LTP] [PATCH 08/19] Unify error handling in lib/tst_mkfs.c Martin Doucha
@ 2020-10-26 16:47 ` Martin Doucha
2020-11-10 15:17 ` Cyril Hrubis
2020-10-26 16:47 ` [LTP] [PATCH 10/19] Unify error handling in lib/tst_net.c Martin Doucha
` (10 subsequent siblings)
19 siblings, 1 reply; 50+ messages in thread
From: Martin Doucha @ 2020-10-26 16:47 UTC (permalink / raw)
To: ltp
- Properly format caller file:line location
Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
lib/tst_checkpoint.c | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/lib/tst_checkpoint.c b/lib/tst_checkpoint.c
index 5e5b11496..9e9dcf9e6 100644
--- a/lib/tst_checkpoint.c
+++ b/lib/tst_checkpoint.c
@@ -43,9 +43,8 @@ void tst_checkpoint_init(const char *file, const int lineno,
unsigned int page_size;
if (tst_futexes) {
- tst_brkm(TBROK, cleanup_fn,
- "%s: %d checkpoints already initialized",
- file, lineno);
+ tst_brkm_(file, lineno, TBROK, cleanup_fn,
+ "checkpoints already initialized");
return;
}
@@ -61,9 +60,9 @@ void tst_checkpoint_init(const char *file, const int lineno,
* the init as a first function with NULL as cleanup function.
*/
if (cleanup_fn && !tst_tmpdir_created()) {
- tst_brkm(TBROK, cleanup_fn,
- "%s:%d You have to create test temporary directory "
- "first (call tst_tmpdir())", file, lineno);
+ tst_brkm_(file, lineno, TBROK, cleanup_fn,
+ "You have to create test temporary directory "
+ "first (call tst_tmpdir())");
return;
}
@@ -144,9 +143,9 @@ void tst_safe_checkpoint_wait(const char *file, const int lineno,
ret = tst_checkpoint_wait(id, msec_timeout);
if (ret) {
- tst_brkm(TBROK | TERRNO, cleanup_fn,
- "%s:%d: tst_checkpoint_wait(%u, %i)",
- file, lineno, id, msec_timeout);
+ tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+ "tst_checkpoint_wait(%u, %i) failed", id,
+ msec_timeout);
}
}
@@ -157,8 +156,8 @@ void tst_safe_checkpoint_wake(const char *file, const int lineno,
int ret = tst_checkpoint_wake(id, nr_wake, DEFAULT_MSEC_TIMEOUT);
if (ret) {
- tst_brkm(TBROK | TERRNO, cleanup_fn,
- "%s:%d: tst_checkpoint_wake(%u, %u, %i)",
- file, lineno, id, nr_wake, DEFAULT_MSEC_TIMEOUT);
+ tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+ "tst_checkpoint_wake(%u, %u, %i) failed", id, nr_wake,
+ DEFAULT_MSEC_TIMEOUT);
}
}
--
2.28.0
^ permalink raw reply related [flat|nested] 50+ messages in thread* [LTP] [PATCH 10/19] Unify error handling in lib/tst_net.c
2020-10-26 16:47 [LTP] [PATCH 00/19] Unify error handling in LTP library Martin Doucha
` (8 preceding siblings ...)
2020-10-26 16:47 ` [LTP] [PATCH 09/19] Unify error handling in lib/tst_checkpoint.c Martin Doucha
@ 2020-10-26 16:47 ` Martin Doucha
2020-11-10 13:20 ` Cyril Hrubis
2020-10-26 16:47 ` [LTP] [PATCH 11/19] Unify error handling in lib/tst_fs_setup.c Martin Doucha
` (9 subsequent siblings)
19 siblings, 1 reply; 50+ messages in thread
From: Martin Doucha @ 2020-10-26 16:47 UTC (permalink / raw)
To: ltp
- Properly format caller file:line location
Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
lib/tst_net.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/lib/tst_net.c b/lib/tst_net.c
index 8a589b0ad..de343bb39 100644
--- a/lib/tst_net.c
+++ b/lib/tst_net.c
@@ -212,10 +212,11 @@ void safe_getaddrinfo(const char *file, const int lineno, const char *src_addr,
{
int err = getaddrinfo(src_addr, port, hints, addr_info);
- if (err)
- tst_brk(TBROK, "%s:%d: getaddrinfo failed, %s", file, lineno,
- gai_strerror(err));
+ if (err) {
+ tst_brk_(file, lineno, TBROK, "getaddrinfo failed, %s",
+ gai_strerror(err));
+ }
if (!*addr_info)
- tst_brk(TBROK, "%s:%d: failed to get the address", file, lineno);
+ tst_brk_(file, lineno, TBROK, "failed to get the address");
}
--
2.28.0
^ permalink raw reply related [flat|nested] 50+ messages in thread* [LTP] [PATCH 11/19] Unify error handling in lib/tst_fs_setup.c
2020-10-26 16:47 [LTP] [PATCH 00/19] Unify error handling in LTP library Martin Doucha
` (9 preceding siblings ...)
2020-10-26 16:47 ` [LTP] [PATCH 10/19] Unify error handling in lib/tst_net.c Martin Doucha
@ 2020-10-26 16:47 ` Martin Doucha
2020-10-27 13:12 ` Yang Xu
2020-10-26 16:47 ` [LTP] [PATCH 12/19] Unify error handling in include/tst_safe_clocks.h Martin Doucha
` (8 subsequent siblings)
19 siblings, 1 reply; 50+ messages in thread
From: Martin Doucha @ 2020-10-26 16:47 UTC (permalink / raw)
To: ltp
- Properly format caller file:line location
Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
lib/tst_fs_setup.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/lib/tst_fs_setup.c b/lib/tst_fs_setup.c
index 54ea37077..a2dacd2ad 100644
--- a/lib/tst_fs_setup.c
+++ b/lib/tst_fs_setup.c
@@ -36,11 +36,11 @@ 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");
--
2.28.0
^ permalink raw reply related [flat|nested] 50+ messages in thread* [LTP] [PATCH 11/19] Unify error handling in lib/tst_fs_setup.c
2020-10-26 16:47 ` [LTP] [PATCH 11/19] Unify error handling in lib/tst_fs_setup.c Martin Doucha
@ 2020-10-27 13:12 ` Yang Xu
0 siblings, 0 replies; 50+ messages in thread
From: Yang Xu @ 2020-10-27 13:12 UTC (permalink / raw)
To: ltp
HI Martin
> - Properly format caller file:line location
>
> Signed-off-by: Martin Doucha<mdoucha@suse.cz>
> ---
> lib/tst_fs_setup.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/lib/tst_fs_setup.c b/lib/tst_fs_setup.c
> index 54ea37077..a2dacd2ad 100644
> --- a/lib/tst_fs_setup.c
> +++ b/lib/tst_fs_setup.c
> @@ -36,11 +36,11 @@ 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");
We should also use tst_brk_ to replace this.
^ permalink raw reply [flat|nested] 50+ messages in thread
* [LTP] [PATCH 12/19] Unify error handling in include/tst_safe_clocks.h
2020-10-26 16:47 [LTP] [PATCH 00/19] Unify error handling in LTP library Martin Doucha
` (10 preceding siblings ...)
2020-10-26 16:47 ` [LTP] [PATCH 11/19] Unify error handling in lib/tst_fs_setup.c Martin Doucha
@ 2020-10-26 16:47 ` Martin Doucha
2020-11-10 13:13 ` Cyril Hrubis
2020-10-26 16:47 ` [LTP] [PATCH 13/19] Move executable code out of tst_safe_macros.h Martin Doucha
` (7 subsequent siblings)
19 siblings, 1 reply; 50+ messages in thread
From: Martin Doucha @ 2020-10-26 16:47 UTC (permalink / raw)
To: ltp
- 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>
---
include/tst_safe_clocks.h | 48 +++++++++++++++++++++++++++------------
1 file changed, 33 insertions(+), 15 deletions(-)
diff --git a/include/tst_safe_clocks.h b/include/tst_safe_clocks.h
index 5909f4083..5661ce57b 100644
--- a/include/tst_safe_clocks.h
+++ b/include/tst_safe_clocks.h
@@ -15,44 +15,62 @@
#include "lapi/syscalls.h"
#include "lapi/posix_clocks.h"
-static inline void safe_clock_getres(const char *file, const int lineno,
+static inline int safe_clock_getres(const char *file, const int lineno,
clockid_t clk_id, struct timespec *res)
{
int rval;
rval = clock_getres(clk_id, res);
- if (rval != 0) {
- tst_brk(TBROK | TERRNO,
- "%s:%d clock_getres(%s) failed",
- file, lineno, tst_clock_name(clk_id));
+
+ if (rval == -1) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "clock_getres(%s) failed", tst_clock_name(clk_id));
+ } else if (rval) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "Invalid clock_getres(%s) return value %d",
+ tst_clock_name(clk_id), rval);
}
+
+ return rval;
}
-static inline void safe_clock_gettime(const char *file, const int lineno,
+static inline int safe_clock_gettime(const char *file, const int lineno,
clockid_t clk_id, struct timespec *tp)
{
int rval;
rval = clock_gettime(clk_id, tp);
- if (rval != 0) {
- tst_brk(TBROK | TERRNO,
- "%s:%d clock_gettime(%s) failed",
- file, lineno, tst_clock_name(clk_id));
+
+ if (rval == -1) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "clock_gettime(%s) failed", tst_clock_name(clk_id));
+ } else if (rval) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "Invalid clock_gettime(%s) return value %d",
+ tst_clock_name(clk_id), rval);
}
+
+ return rval;
}
-static inline void safe_clock_settime(const char *file, const int lineno,
+static inline int safe_clock_settime(const char *file, const int lineno,
clockid_t clk_id, struct timespec *tp)
{
int rval;
rval = clock_settime(clk_id, tp);
- if (rval != 0) {
- tst_brk(TBROK | TERRNO,
- "%s:%d clock_gettime(%s) failed",
- file, lineno, tst_clock_name(clk_id));
+
+ if (rval == -1) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "clock_gettime(%s) failed", tst_clock_name(clk_id));
+ } else if (rval) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "Invalid clock_gettime(%s) return value %d",
+ tst_clock_name(clk_id), rval);
}
+
+ return rval;
}
static inline int safe_timer_create(const char *file, const int lineno,
--
2.28.0
^ permalink raw reply related [flat|nested] 50+ messages in thread* [LTP] [PATCH 13/19] Move executable code out of tst_safe_macros.h
2020-10-26 16:47 [LTP] [PATCH 00/19] Unify error handling in LTP library Martin Doucha
` (11 preceding siblings ...)
2020-10-26 16:47 ` [LTP] [PATCH 12/19] Unify error handling in include/tst_safe_clocks.h Martin Doucha
@ 2020-10-26 16:47 ` Martin Doucha
2020-10-26 16:47 ` [LTP] [PATCH 14/19] Unify error handling in moved functions Martin Doucha
` (6 subsequent siblings)
19 siblings, 0 replies; 50+ messages in thread
From: Martin Doucha @ 2020-10-26 16:47 UTC (permalink / raw)
To: ltp
Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
include/tst_safe_macros.h | 44 +++++----------------------------------
lib/tst_safe_macros.c | 44 +++++++++++++++++++++++++++++++++++++++
2 files changed, 49 insertions(+), 39 deletions(-)
diff --git a/include/tst_safe_macros.h b/include/tst_safe_macros.h
index 42d923370..29ac72568 100644
--- a/include/tst_safe_macros.h
+++ b/include/tst_safe_macros.h
@@ -45,19 +45,8 @@ int safe_chroot(const char *file, const int lineno, const char *path);
#define SAFE_DIRNAME(path) \
safe_dirname(__FILE__, __LINE__, NULL, (path))
-static inline int safe_dup(const char *file, const int lineno,
- int oldfd)
-{
- int rval;
-
- rval = dup(oldfd);
- if (rval == -1) {
- tst_brk_(file, lineno, TBROK | TERRNO,
- "dup(%i) failed", oldfd);
- }
+int safe_dup(const char *file, const int lineno, int oldfd);
- return rval;
-}
#define SAFE_DUP(oldfd) \
safe_dup(__FILE__, __LINE__, (oldfd))
@@ -407,21 +396,8 @@ static inline int safe_setrlimit(const char *file, const int lineno,
safe_setrlimit(__FILE__, __LINE__, (resource), (rlim))
typedef void (*sighandler_t)(int);
-static inline sighandler_t safe_signal(const char *file, const int lineno,
- int signum, sighandler_t handler)
-{
- sighandler_t rval;
-
- rval = signal(signum, handler);
-
- if (rval == SIG_ERR) {
- tst_brk_(file, lineno, TBROK | TERRNO,
- "signal(%d,%p) failed",
- signum, handler);
- }
-
- return rval;
-}
+sighandler_t safe_signal(const char *file, const int lineno,
+ int signum, sighandler_t handler);
#define SAFE_SIGNAL(signum, handler) \
safe_signal(__FILE__, __LINE__, (signum), (handler))
@@ -569,19 +545,9 @@ int safe_unshare(const char *file, const int lineno, int flags);
int safe_setns(const char *file, const int lineno, int fd, int nstype);
#define SAFE_SETNS(fd, nstype) safe_setns(__FILE__, __LINE__, (fd), (nstype))
-static inline void safe_cmd(const char *file, const int lineno, const char *const argv[],
- const char *stdout_path, const char *stderr_path)
-{
- int rval;
+void safe_cmd(const char *file, const int lineno, const char *const argv[],
+ const char *stdout_path, const char *stderr_path);
- switch ((rval = tst_cmd(argv, stdout_path, stderr_path,
- TST_CMD_PASS_RETVAL | TST_CMD_TCONF_ON_MISSING))) {
- case 0:
- break;
- default:
- tst_brk(TBROK, "%s:%d: %s failed (%d)", file, lineno, argv[0], rval);
- }
-}
#define SAFE_CMD(argv, stdout_path, stderr_path) \
safe_cmd(__FILE__, __LINE__, (argv), (stdout_path), (stderr_path))
/*
diff --git a/lib/tst_safe_macros.c b/lib/tst_safe_macros.c
index dd7f604eb..5c51e0872 100644
--- a/lib/tst_safe_macros.c
+++ b/lib/tst_safe_macros.c
@@ -397,3 +397,47 @@ int safe_pipe2(const char *file, const int lineno, int fildes[2], int flags)
return ret;
}
+
+int safe_dup(const char *file, const int lineno, int oldfd)
+{
+ int rval;
+
+ rval = dup(oldfd);
+ if (rval == -1) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "dup(%i) failed", oldfd);
+ }
+
+ return rval;
+}
+
+sighandler_t safe_signal(const char *file, const int lineno,
+ int signum, sighandler_t handler)
+{
+ sighandler_t rval;
+
+ rval = signal(signum, handler);
+
+ if (rval == SIG_ERR) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "signal(%d,%p) failed",
+ signum, handler);
+ }
+
+ return rval;
+}
+
+void safe_cmd(const char *file, const int lineno, const char *const argv[],
+ const char *stdout_path, const char *stderr_path)
+{
+ int rval;
+
+ switch ((rval = tst_cmd(argv, stdout_path, stderr_path,
+ TST_CMD_PASS_RETVAL | TST_CMD_TCONF_ON_MISSING))) {
+ case 0:
+ break;
+ default:
+ tst_brk(TBROK, "%s:%d: %s failed (%d)", file, lineno, argv[0],
+ rval);
+ }
+}
--
2.28.0
^ permalink raw reply related [flat|nested] 50+ messages in thread* [LTP] [PATCH 14/19] Unify error handling in moved functions
2020-10-26 16:47 [LTP] [PATCH 00/19] Unify error handling in LTP library Martin Doucha
` (12 preceding siblings ...)
2020-10-26 16:47 ` [LTP] [PATCH 13/19] Move executable code out of tst_safe_macros.h Martin Doucha
@ 2020-10-26 16:47 ` Martin Doucha
2020-11-10 12:18 ` Cyril Hrubis
2020-10-26 16:47 ` [LTP] [PATCH 15/19] Unify error handling in include/tst_safe_macros.h Martin Doucha
` (5 subsequent siblings)
19 siblings, 1 reply; 50+ messages in thread
From: Martin Doucha @ 2020-10-26 16:47 UTC (permalink / raw)
To: ltp
- Properly format caller file:line location
- Pedantically check invalid syscall return values
Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
lib/tst_safe_macros.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/lib/tst_safe_macros.c b/lib/tst_safe_macros.c
index 5c51e0872..aa03a6d5c 100644
--- a/lib/tst_safe_macros.c
+++ b/lib/tst_safe_macros.c
@@ -403,9 +403,13 @@ int safe_dup(const char *file, const int lineno, int oldfd)
int rval;
rval = dup(oldfd);
+
if (rval == -1) {
tst_brk_(file, lineno, TBROK | TERRNO,
- "dup(%i) failed", oldfd);
+ "dup(%i) failed", oldfd);
+ } else if (rval < 0) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "Invalid dup(%i) return value %d", oldfd, rval);
}
return rval;
@@ -437,7 +441,6 @@ void safe_cmd(const char *file, const int lineno, const char *const argv[],
case 0:
break;
default:
- tst_brk(TBROK, "%s:%d: %s failed (%d)", file, lineno, argv[0],
- rval);
+ tst_brk_(file, lineno, TBROK, "%s failed (%d)", argv[0], rval);
}
}
--
2.28.0
^ permalink raw reply related [flat|nested] 50+ messages in thread* [LTP] [PATCH 15/19] Unify error handling in include/tst_safe_macros.h
2020-10-26 16:47 [LTP] [PATCH 00/19] Unify error handling in LTP library Martin Doucha
` (13 preceding siblings ...)
2020-10-26 16:47 ` [LTP] [PATCH 14/19] Unify error handling in moved functions Martin Doucha
@ 2020-10-26 16:47 ` Martin Doucha
2020-11-10 11:37 ` Cyril Hrubis
2020-10-26 16:47 ` [LTP] [PATCH 16/19] Unify error handling in include/tst_safe_posix_ipc.h Martin Doucha
` (4 subsequent siblings)
19 siblings, 1 reply; 50+ messages in thread
From: Martin Doucha @ 2020-10-26 16:47 UTC (permalink / raw)
To: ltp
- Pedantically check invalid syscall return values
Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
include/tst_safe_macros.h | 56 ++++++++++++++++++++++++++++++---------
1 file changed, 44 insertions(+), 12 deletions(-)
diff --git a/include/tst_safe_macros.h b/include/tst_safe_macros.h
index 29ac72568..ee3df4142 100644
--- a/include/tst_safe_macros.h
+++ b/include/tst_safe_macros.h
@@ -245,10 +245,14 @@ static inline int safe_ftruncate(const char *file, const int lineno,
int rval;
rval = ftruncate(fd, length);
+
if (rval == -1) {
tst_brk_(file, lineno, TBROK | TERRNO,
- "ftruncate(%d,%ld) failed",
- fd, (long)length);
+ "ftruncate(%d,%ld) failed", fd, (long)length);
+ } else if (rval) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "Invalid ftruncate(%d,%ld) return value %d", fd,
+ (long)length, rval);
}
return rval;
@@ -262,10 +266,14 @@ static inline int safe_truncate(const char *file, const int lineno,
int rval;
rval = truncate(path, length);
+
if (rval == -1) {
tst_brk_(file, lineno, TBROK | TERRNO,
- "truncate(%s,%ld) failed",
- path, (long)length);
+ "truncate(%s,%ld) failed", path, (long)length);
+ } else if (rval) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "Invalid truncate(%s,%ld) return value %d", path,
+ (long)length, rval);
}
return rval;
@@ -282,7 +290,11 @@ static inline int safe_stat(const char *file, const int lineno,
if (rval == -1) {
tst_brk_(file, lineno, TBROK | TERRNO,
- "stat(%s,%p) failed", path, buf);
+ "stat(%s,%p) failed", path, buf);
+ } else if (rval) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "Invalid stat(%s,%p) return value %d", path, buf,
+ rval);
}
return rval;
@@ -300,6 +312,9 @@ static inline int safe_fstat(const char *file, const int lineno,
if (rval == -1) {
tst_brk_(file, lineno, TBROK | TERRNO,
"fstat(%d,%p) failed", fd, buf);
+ } else if (rval) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "Invalid fstat(%d,%p) return value %d", fd, buf, rval);
}
return rval;
@@ -317,6 +332,10 @@ static inline int safe_lstat(const char *file, const int lineno,
if (rval == -1) {
tst_brk_(file, lineno, TBROK | TERRNO,
"lstat(%s,%p) failed", path, buf);
+ } else if (rval) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "Invalid lstat(%s,%p) return value %d", path, buf,
+ rval);
}
return rval;
@@ -333,7 +352,11 @@ static inline int safe_statfs(const char *file, const int lineno,
if (rval == -1) {
tst_brk_(file, lineno, TBROK | TERRNO,
- "statfs(%s,%p) failed", path, buf);
+ "statfs(%s,%p) failed", path, buf);
+ } else if (rval) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "Invalid statfs(%s,%p) return value %d", path, buf,
+ rval);
}
return rval;
@@ -350,8 +373,11 @@ static inline off_t safe_lseek(const char *file, const int lineno,
if (rval == (off_t) -1) {
tst_brk_(file, lineno, TBROK | TERRNO,
- "lseek(%d,%ld,%d) failed",
- fd, (long)offset, whence);
+ "lseek(%d,%ld,%d) failed", fd, (long)offset, whence);
+ } else if (rval < 0) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "Invalid lseek(%d,%ld,%d) return value %ld", fd,
+ (long)offset, whence, (long)rval);
}
return rval;
@@ -368,8 +394,11 @@ static inline int safe_getrlimit(const char *file, const int lineno,
if (rval == -1) {
tst_brk_(file, lineno, TBROK | TERRNO,
- "getrlimit(%d,%p) failed",
- resource, rlim);
+ "getrlimit(%d,%p) failed", resource, rlim);
+ } else if (rval) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "Invalid getrlimit(%d,%p) return value %d", resource,
+ rlim, rval);
}
return rval;
@@ -386,8 +415,11 @@ static inline int safe_setrlimit(const char *file, const int lineno,
if (rval == -1) {
tst_brk_(file, lineno, TBROK | TERRNO,
- "setrlimit(%d,%p) failed",
- resource, rlim);
+ "setrlimit(%d,%p) failed", resource, rlim);
+ } else if (rval) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "Invalid setrlimit(%d,%p) return value %d", resource,
+ rlim, rval);
}
return rval;
--
2.28.0
^ permalink raw reply related [flat|nested] 50+ messages in thread* [LTP] [PATCH 16/19] Unify error handling in include/tst_safe_posix_ipc.h
2020-10-26 16:47 [LTP] [PATCH 00/19] Unify error handling in LTP library Martin Doucha
` (14 preceding siblings ...)
2020-10-26 16:47 ` [LTP] [PATCH 15/19] Unify error handling in include/tst_safe_macros.h Martin Doucha
@ 2020-10-26 16:47 ` Martin Doucha
2020-11-06 15:43 ` Cyril Hrubis
2020-10-26 16:47 ` [LTP] [PATCH 17/19] Unify error handling in include/tst_safe_prw.h Martin Doucha
` (3 subsequent siblings)
19 siblings, 1 reply; 50+ messages in thread
From: Martin Doucha @ 2020-10-26 16:47 UTC (permalink / raw)
To: ltp
- Properly format caller file:line location
Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
include/tst_safe_posix_ipc.h | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/include/tst_safe_posix_ipc.h b/include/tst_safe_posix_ipc.h
index d74ef4ee8..b60c12c9e 100644
--- a/include/tst_safe_posix_ipc.h
+++ b/include/tst_safe_posix_ipc.h
@@ -36,9 +36,11 @@ static inline int safe_mq_open(const char *file, const int lineno,
va_end(ap);
rval = mq_open(pathname, oflags, mode, attr);
+
if (rval == -1) {
- tst_brk(TBROK | TERRNO, "%s:%d: mq_open(%s,%d,0%o,%p) failed",
- file, lineno, pathname, oflags, mode, attr);
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "mq_open(%s,%d,%04o,%p) failed", pathname, oflags,
+ mode, attr);
}
return rval;
--
2.28.0
^ permalink raw reply related [flat|nested] 50+ messages in thread* [LTP] [PATCH 17/19] Unify error handling in include/tst_safe_prw.h
2020-10-26 16:47 [LTP] [PATCH 00/19] Unify error handling in LTP library Martin Doucha
` (15 preceding siblings ...)
2020-10-26 16:47 ` [LTP] [PATCH 16/19] Unify error handling in include/tst_safe_posix_ipc.h Martin Doucha
@ 2020-10-26 16:47 ` Martin Doucha
2020-11-06 15:39 ` Cyril Hrubis
2020-10-26 16:47 ` [LTP] [PATCH 18/19] Unify error handling in lib/tst_resource.c Martin Doucha
` (2 subsequent siblings)
19 siblings, 1 reply; 50+ messages in thread
From: Martin Doucha @ 2020-10-26 16:47 UTC (permalink / raw)
To: ltp
- Pedantically check syscall return values
Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
include/tst_safe_prw.h | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/include/tst_safe_prw.h b/include/tst_safe_prw.h
index 01a684da3..2e506cb41 100644
--- a/include/tst_safe_prw.h
+++ b/include/tst_safe_prw.h
@@ -15,8 +15,12 @@ static inline ssize_t safe_pread(const char *file, const int lineno,
if (rval == -1 || (len_strict && (size_t)rval != nbyte)) {
tst_brk_(file, lineno, TBROK | TERRNO,
- "pread(%d,%p,%zu,%lld) failed",
- fildes, buf, nbyte, (long long)offset);
+ "pread(%d,%p,%zu,%lld) failed",
+ fildes, buf, nbyte, (long long)offset);
+ } else if (rval < 0) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "Invalid pread(%d,%p,%zu,%lld) return value %zd",
+ fildes, buf, nbyte, (long long)offset, rval);
}
return rval;
@@ -34,8 +38,12 @@ static inline ssize_t safe_pwrite(const char *file, const int lineno,
rval = pwrite(fildes, buf, nbyte, offset);
if (rval == -1 || (len_strict && (size_t)rval != nbyte)) {
tst_brk_(file, lineno, TBROK | TERRNO,
- "pwrite(%d,%p,%zu,%lld) failed",
- fildes, buf, nbyte, (long long)offset);
+ "pwrite(%d,%p,%zu,%lld) failed",
+ fildes, buf, nbyte, (long long)offset);
+ } else if (rval < 0) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "Invalid pwrite(%d,%p,%zu,%lld) return value %zd",
+ fildes, buf, nbyte, (long long)offset, rval);
}
return rval;
--
2.28.0
^ permalink raw reply related [flat|nested] 50+ messages in thread* [LTP] [PATCH 18/19] Unify error handling in lib/tst_resource.c
2020-10-26 16:47 [LTP] [PATCH 00/19] Unify error handling in LTP library Martin Doucha
` (16 preceding siblings ...)
2020-10-26 16:47 ` [LTP] [PATCH 17/19] Unify error handling in include/tst_safe_prw.h Martin Doucha
@ 2020-10-26 16:47 ` Martin Doucha
2020-11-06 13:05 ` Cyril Hrubis
2020-10-26 16:47 ` [LTP] [PATCH 19/19] Unify error handling in include/lapi/safe_rt_signal.h Martin Doucha
2020-10-27 13:32 ` [LTP] [PATCH 00/19] Unify error handling in LTP library Yang Xu
19 siblings, 1 reply; 50+ messages in thread
From: Martin Doucha @ 2020-10-26 16:47 UTC (permalink / raw)
To: ltp
- Properly format caller file:line location
Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
lib/tst_resource.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/lib/tst_resource.c b/lib/tst_resource.c
index 0b9b381f1..c35d05a25 100644
--- a/lib/tst_resource.c
+++ b/lib/tst_resource.c
@@ -102,9 +102,8 @@ void tst_resource_copy(const char *file, const int lineno,
const char *filename, const char *dest)
{
if (!tst_tmpdir_created()) {
- tst_brkm(TBROK, cleanup_fn,
- "Temporary directory doesn't exist at %s:%d",
- file, lineno);
+ tst_brkm_(file, lineno, TBROK, cleanup_fn,
+ "Temporary directory doesn't exist");
return;
}
@@ -133,6 +132,6 @@ void tst_resource_copy(const char *file, const int lineno,
if (file_copy(file, lineno, cleanup_fn, startwd, filename, dest))
return;
- tst_brkm(TBROK, cleanup_fn, "Failed to copy resource '%s' at %s:%d",
- filename, file, lineno);
+ tst_brkm_(file, lineno, TBROK, cleanup_fn,
+ "Failed to copy resource '%s'", filename);
}
--
2.28.0
^ permalink raw reply related [flat|nested] 50+ messages in thread* [LTP] [PATCH 19/19] Unify error handling in include/lapi/safe_rt_signal.h
2020-10-26 16:47 [LTP] [PATCH 00/19] Unify error handling in LTP library Martin Doucha
` (17 preceding siblings ...)
2020-10-26 16:47 ` [LTP] [PATCH 18/19] Unify error handling in lib/tst_resource.c Martin Doucha
@ 2020-10-26 16:47 ` Martin Doucha
2020-11-06 12:57 ` Cyril Hrubis
2020-10-27 13:32 ` [LTP] [PATCH 00/19] Unify error handling in LTP library Yang Xu
19 siblings, 1 reply; 50+ messages in thread
From: Martin Doucha @ 2020-10-26 16:47 UTC (permalink / raw)
To: ltp
- Properly format caller file:line location
- Pedantically check invalid syscall return values
Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
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..f218f19a2 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
^ permalink raw reply related [flat|nested] 50+ messages in thread* [LTP] [PATCH 19/19] Unify error handling in include/lapi/safe_rt_signal.h
2020-10-26 16:47 ` [LTP] [PATCH 19/19] Unify error handling in include/lapi/safe_rt_signal.h Martin Doucha
@ 2020-11-06 12:57 ` Cyril Hrubis
0 siblings, 0 replies; 50+ messages in thread
From: Cyril Hrubis @ 2020-11-06 12:57 UTC (permalink / raw)
To: ltp
Hi!
> + 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);
The LKML coding style prefers not to break strings even if they are over
80 characters.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 50+ messages in thread
* [LTP] [PATCH 00/19] Unify error handling in LTP library
2020-10-26 16:47 [LTP] [PATCH 00/19] Unify error handling in LTP library Martin Doucha
` (18 preceding siblings ...)
2020-10-26 16:47 ` [LTP] [PATCH 19/19] Unify error handling in include/lapi/safe_rt_signal.h Martin Doucha
@ 2020-10-27 13:32 ` Yang Xu
19 siblings, 0 replies; 50+ messages in thread
From: Yang Xu @ 2020-10-27 13:32 UTC (permalink / raw)
To: ltp
Hi Martin
I have seen the patchset and send some comments on 2,3,5,11 patch,
for the patchset, you can add Acked-by.
Acked-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
Best Regard
Yang Xu
> LTP helper functions, mainly safe syscalls, often report two different source
> code locations in error messages and sometimes handle syscall return values
> incorrectly. This patchset unifies source code location formatting to only
> show the calling line in the test program and fixes invalid return value
> handling. Safe syscalls now make pedantic distinction between common errors
> and invalid return values where applicable.
>
> Some safe syscalls returned no value so they were not usable in test cleanup.
> This is also fixed along with potential control flow issues when tst_brk()
> does not immediately terminate the program.
>
> Martin Doucha (19):
> Unify error handling in lib/tst_safe_macros.c
> Unify error handling in lib/tst_safe_sysv_ipc.c
> Unify error handling in lib/tst_safe_timerfd.c
> Unify error handling in lib/safe_file_ops.c
> Unify error handling in lib/safe_macros.c
> Unify error handling in lib/safe_net.c
> Unify error handling in lib/safe_stdio.c
> Unify error handling in lib/tst_mkfs.c
> Unify error handling in lib/tst_checkpoint.c
> Unify error handling in lib/tst_net.c
> Unify error handling in lib/tst_fs_setup.c
> Unify error handling in include/tst_safe_clocks.h
> Move executable code out of tst_safe_macros.h
> Unify error handling in moved functions
> Unify error handling in include/tst_safe_macros.h
> Unify error handling in include/tst_safe_posix_ipc.h
> Unify error handling in include/tst_safe_prw.h
> Unify error handling in lib/tst_resource.c
> Unify error handling in include/lapi/safe_rt_signal.h
>
> include/lapi/safe_rt_signal.h | 25 +-
> include/safe_file_ops_fn.h | 8 +-
> include/tst_safe_clocks.h | 48 ++-
> include/tst_safe_macros.h | 130 ++++----
> include/tst_safe_posix_ipc.h | 6 +-
> include/tst_safe_prw.h | 16 +-
> lib/safe_file_ops.c | 207 +++++++-----
> lib/safe_macros.c | 602 ++++++++++++++++++++++------------
> lib/safe_net.c | 262 +++++++++------
> lib/safe_stdio.c | 34 +-
> lib/tst_checkpoint.c | 23 +-
> lib/tst_fs_setup.c | 8 +-
> lib/tst_mkfs.c | 36 +-
> lib/tst_net.c | 9 +-
> lib/tst_resource.c | 9 +-
> lib/tst_safe_macros.c | 201 ++++++++++--
> lib/tst_safe_sysv_ipc.c | 79 +++--
> lib/tst_safe_timerfd.c | 32 +-
> 18 files changed, 1100 insertions(+), 635 deletions(-)
>
^ permalink raw reply [flat|nested] 50+ messages in thread