[LTP] [PATCH 1/5] Cleanup && Drop redundant space in safe macros

[LTP] [PATCH 1/5] Cleanup && Drop redundant space in safe macros

[Guangwen Feng]

Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
---
 include/tst_safe_macros.h | 22 +++++++++++-----------
 lib/safe_macros.c         | 22 +++++++++++-----------
 2 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/include/tst_safe_macros.h b/include/tst_safe_macros.h
index e91dbd2..ffb5446 100644
--- a/include/tst_safe_macros.h
+++ b/include/tst_safe_macros.h
@@ -146,7 +146,7 @@ static inline int safe_setpgid(const char *file, const int lineno,
 	rval = setpgid(pid, pgid);
 	if (rval) {
 		tst_brk_(file, lineno, TBROK | TERRNO,
-		         "setpgid(%i, %i) failed", pid, pgid);
+			 "setpgid(%i, %i) failed", pid, pgid);
 	}
 
 	return rval;
@@ -292,8 +292,8 @@ static inline void *safe_mmap(const char *file, const int lineno,
 	rval = mmap(addr, length, prot, flags, fd, offset);
 	if (rval == MAP_FAILED) {
 		tst_brk_(file, lineno, TBROK | TERRNO,
-			"mmap(%p,%zu,%d,%d,%d,%ld) failed",
-			addr, length, prot, flags, fd, (long) offset);
+			 "mmap(%p,%zu,%d,%d,%d,%ld) failed",
+			 addr, length, prot, flags, fd, (long) offset);
 	}
 
 	return rval;
@@ -362,7 +362,7 @@ 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);
+			 "fstat(%d,%p) failed", fd, buf);
 	}
 
 	return rval;
@@ -379,7 +379,7 @@ 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);
+			 "lstat(%s,%p) failed", path, buf);
 	}
 
 	return rval;
@@ -396,8 +396,8 @@ 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);
 	}
 
 	return rval;
@@ -414,8 +414,8 @@ 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);
 	}
 
 	return rval;
@@ -451,8 +451,8 @@ static inline sighandler_t safe_signal(const char *file, const int lineno,
 
 	if (rval == SIG_ERR) {
 		tst_brk_(file, lineno, TBROK | TERRNO,
-			"signal(%d,%p) failed",
-			signum, handler);
+			 "signal(%d,%p) failed",
+			 signum, handler);
 	}
 
 	return rval;
diff --git a/lib/safe_macros.c b/lib/safe_macros.c
index 31dc61c..2db362f 100644
--- a/lib/safe_macros.c
+++ b/lib/safe_macros.c
@@ -368,7 +368,7 @@ int safe_link(const char *file, const int lineno,
 
 	if (rval == -1) {
 		tst_brkm(TBROK | TERRNO, cleanup_fn,
-		         "%s:%d: link(%s,%s) failed",
+			 "%s:%d: link(%s,%s) failed",
 			 file, lineno, oldpath, newpath);
 	}
 
@@ -420,7 +420,7 @@ int safe_symlink(const char *file, const int lineno,
 
 	if (rval == -1) {
 		tst_brkm(TBROK | TERRNO, cleanup_fn,
-		         "%s:%d: symlink(%s,%s) failed",
+			 "%s:%d: symlink(%s,%s) failed",
 			 file, lineno, oldpath, newpath);
 	}
 
@@ -436,7 +436,7 @@ ssize_t safe_write(const char *file, const int lineno, void (cleanup_fn) (void),
 	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);
+			 file, lineno, fildes, buf, rval);
 	}
 
 	return rval;
@@ -566,8 +566,8 @@ int safe_chown(const char *file, const int lineno, void (cleanup_fn)(void),
 
 	if (rval == -1) {
 		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			"%s:%d: chown(%s,%d,%d) failed",
-			file, lineno, path, owner, group);
+			 "%s:%d: chown(%s,%d,%d) failed",
+			 file, lineno, path, owner, group);
 	}
 
 	return rval;
@@ -582,7 +582,7 @@ int safe_fchown(const char *file, const int lineno, void (cleanup_fn)(void),
 
 	if (rval == -1) {
 		tst_brkm(TBROK | TERRNO, cleanup_fn,
-		         "%s:%d: fchown(%d,%d,%d) failed",
+			 "%s:%d: fchown(%d,%d,%d) failed",
 			 file, lineno, fd, owner, group);
 	}
 
@@ -657,7 +657,7 @@ int safe_mkfifo(const char *file, const int lineno,
 
 	if (rval == -1) {
 		tst_brkm(TBROK | TERRNO, cleanup_fn,
-		         "%s:%d: mkfifo(%s, 0%o) failed",
+			 "%s:%d: mkfifo(%s, 0%o) failed",
 			 file, lineno, pathname, mode);
 	}
 
@@ -724,7 +724,7 @@ DIR* safe_opendir(const char *file, const int lineno, void (cleanup_fn)(void),
 
 	if (!rval) {
 		tst_brkm(TBROK | TERRNO, cleanup_fn,
-		         "%s:%d: opendir(%s) failed", file, lineno, name);
+			 "%s:%d: opendir(%s) failed", file, lineno, name);
 	}
 
 	return rval;
@@ -739,7 +739,7 @@ int safe_closedir(const char *file, const int lineno, void (cleanup_fn)(void),
 
 	if (rval) {
 		tst_brkm(TBROK | TERRNO, cleanup_fn,
-		         "%s:%d: closedir(%p) failed", file, lineno, dirp);
+			 "%s:%d: closedir(%p) failed", file, lineno, dirp);
 	}
 
 	return rval;
@@ -756,7 +756,7 @@ struct dirent *safe_readdir(const char *file, const int lineno, void (cleanup_fn
 
 	if (!rval && errno) {
 		tst_brkm(TBROK | TERRNO, cleanup_fn,
-		         "%s:%d: readdir(%p) failed", file, lineno, dirp);
+			 "%s:%d: readdir(%p) failed", file, lineno, dirp);
 	}
 
 	errno = err;
@@ -771,7 +771,7 @@ int safe_getpriority(const char *file, const int lineno, int which, id_t who)
 	rval = getpriority(which, who);
 	if (errno) {
 		tst_brkm(TBROK | TERRNO, NULL,
-		         "%s:%d getpriority(%i, %i) failed",
+			 "%s:%d getpriority(%i, %i) failed",
 			 file, lineno, which, who);
 	}
 
-- 
1.8.4.2

[LTP] [PATCH 2/5] SAFE_MACROS: Add SAFE_SETPRIORITY()

[Guangwen Feng]

Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
---
 include/tst_safe_macros.h |  5 +++++
 lib/safe_macros.c         | 15 +++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/include/tst_safe_macros.h b/include/tst_safe_macros.h
index ffb5446..9fa0e17 100644
--- a/include/tst_safe_macros.h
+++ b/include/tst_safe_macros.h
@@ -471,4 +471,9 @@ int safe_getpriority(const char *file, const int lineno, int which, id_t who);
 #define SAFE_GETPRIORITY(which, who) \
 	safe_getpriority(__FILE__, __LINE__, (which), (who))
 
+int safe_setpriority(const char *file, const int lineno, int which, id_t who,
+	int prio);
+#define SAFE_SETPRIORITY(which, who, prio) \
+	safe_setpriority(__FILE__, __LINE__, (which), (who), (prio))
+
 #endif /* SAFE_MACROS_H__ */
diff --git a/lib/safe_macros.c b/lib/safe_macros.c
index 2db362f..a109270 100644
--- a/lib/safe_macros.c
+++ b/lib/safe_macros.c
@@ -778,3 +778,18 @@ int safe_getpriority(const char *file, const int lineno, int which, id_t who)
 	errno = err;
 	return rval;
 }
+
+int safe_setpriority(const char *file, const int lineno, int which, id_t who,
+	int prio)
+{
+	int rval;
+
+	rval = setpriority(which, who, prio);
+	if (rval) {
+		tst_brkm(TBROK | TERRNO, NULL,
+			 "%s:%d setpriority(%i, %i, %i) failed",
+			 file, lineno, which, who, prio);
+	}
+
+	return rval;
+}
-- 
1.8.4.2

[LTP] [PATCH 3/5] SAFE_MACROS: Add SAFE_SYSTEM()

[Guangwen Feng]

Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
---
 include/tst_safe_macros.h |  4 ++++
 lib/safe_macros.c         | 14 ++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/include/tst_safe_macros.h b/include/tst_safe_macros.h
index 9fa0e17..2566079 100644
--- a/include/tst_safe_macros.h
+++ b/include/tst_safe_macros.h
@@ -476,4 +476,8 @@ int safe_setpriority(const char *file, const int lineno, int which, id_t who,
 #define SAFE_SETPRIORITY(which, who, prio) \
 	safe_setpriority(__FILE__, __LINE__, (which), (who), (prio))
 
+int safe_system(const char *file, const int lineno, const char *cmd);
+#define SAFE_SYSTEM(cmd) \
+	safe_system(__FILE__, __LINE__, (cmd))
+
 #endif /* SAFE_MACROS_H__ */
diff --git a/lib/safe_macros.c b/lib/safe_macros.c
index a109270..1c89b8c 100644
--- a/lib/safe_macros.c
+++ b/lib/safe_macros.c
@@ -793,3 +793,17 @@ int safe_setpriority(const char *file, const int lineno, int which, id_t who,
 
 	return rval;
 }
+
+int safe_system(const char *file, const int lineno, const char *cmd)
+{
+	int rval;
+
+	rval = system(cmd);
+	if (rval == -1 || !WIFEXITED(rval) || WEXITSTATUS(rval)) {
+		tst_brkm(TBROK | TERRNO, NULL,
+			 "%s:%d system(%s) failed",
+			 file, lineno, cmd);
+	}
+
+	return rval;
+}
-- 
1.8.4.2

[LTP] [PATCH 3/5] SAFE_MACROS: Add SAFE_SYSTEM()

[Cyril Hrubis]

Hi!
NACK to this one. We already have tst_run_cmd() which is way better for
this purpose.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 3/5] SAFE_MACROS: Add SAFE_SYSTEM()

[Guangwen Feng]

Hi!

On 12/05/2016 10:33 PM, Cyril Hrubis wrote:
> Hi!
> NACK to this one. We already have tst_run_cmd() which is way better for
> this purpose.

I got it, will use tst_run_cmd() instead, thanks very much.

Best Regards,
Guangwen Feng

[LTP] [PATCH 4/5] SAFE_MACROS: Add SAFE_PRCTL()

[Guangwen Feng]

Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
---
 include/tst_safe_macros.h |  4 ++++
 lib/safe_macros.c         | 20 ++++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/include/tst_safe_macros.h b/include/tst_safe_macros.h
index 2566079..bd60b40 100644
--- a/include/tst_safe_macros.h
+++ b/include/tst_safe_macros.h
@@ -480,4 +480,8 @@ int safe_system(const char *file, const int lineno, const char *cmd);
 #define SAFE_SYSTEM(cmd) \
 	safe_system(__FILE__, __LINE__, (cmd))
 
+int safe_prctl(const char *file, const int lineno, int opt, ...);
+#define SAFE_PRCTL(opt, ...) \
+	safe_prctl(__FILE__, __LINE__, (opt), ##__VA_ARGS__)
+
 #endif /* SAFE_MACROS_H__ */
diff --git a/lib/safe_macros.c b/lib/safe_macros.c
index 1c89b8c..c8c5383 100644
--- a/lib/safe_macros.c
+++ b/lib/safe_macros.c
@@ -807,3 +807,23 @@ int safe_system(const char *file, const int lineno, const char *cmd)
 
 	return rval;
 }
+
+int safe_prctl(const char *file, const int lineno, int opt, ...)
+{
+	va_list ap;
+	int rval;
+	unsigned long arg;
+
+	va_start(ap, opt);
+	arg = va_arg(ap, unsigned long);
+	va_end(ap);
+
+	rval = prctl(opt, arg);
+	if (rval < 0) {
+		tst_brkm(TBROK | TERRNO, NULL,
+			 "%s:%d prctl(%i, %lu) failed",
+			 file, lineno, opt, arg);
+	}
+
+	return rval;
+}
-- 
1.8.4.2

[LTP] [PATCH 4/5] SAFE_MACROS: Add SAFE_PRCTL()

[Cyril Hrubis]

Hi!
> +int safe_prctl(const char *file, const int lineno, int opt, ...);
> +#define SAFE_PRCTL(opt, ...) \
> +	safe_prctl(__FILE__, __LINE__, (opt), ##__VA_ARGS__)
> +
>  #endif /* SAFE_MACROS_H__ */
> diff --git a/lib/safe_macros.c b/lib/safe_macros.c
> index 1c89b8c..c8c5383 100644
> --- a/lib/safe_macros.c
> +++ b/lib/safe_macros.c
> @@ -807,3 +807,23 @@ int safe_system(const char *file, const int lineno, const char *cmd)
>  
>  	return rval;
>  }
> +
> +int safe_prctl(const char *file, const int lineno, int opt, ...)
> +{
> +	va_list ap;
> +	int rval;
> +	unsigned long arg;
> +
> +	va_start(ap, opt);
> +	arg = va_arg(ap, unsigned long);
> +	va_end(ap);

This is broken by design. The prctl() can have up to four long arguments
but here we use only the first one unconditionally.


I'm not sure how we can fix this easily. Either we can count number of
arguments passed to the SAFE_PRCTL() by hacky preprocessor counting[1]
and pass it as a first agument.

Or we can switch on the opt to figure out the number of expected arguments,
which is ugly as well, but we can at least start small by adding only
opts that we use.

[1]
https://groups.google.com/forum/?_escaped_fragment_=topic/comp.std.c/d-6Mj5Lko_s#!topic/comp.std.c/d-6Mj5Lko_s

> +	rval = prctl(opt, arg);
> +	if (rval < 0) {
> +		tst_brkm(TBROK | TERRNO, NULL,
> +			 "%s:%d prctl(%i, %lu) failed",
> +			 file, lineno, opt, arg);
> +	}
> +
> +	return rval;
> +}
> -- 
> 1.8.4.2
> 
> 
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 4/5] SAFE_MACROS: Add SAFE_PRCTL()

[Guangwen Feng]

Hi!

On 12/05/2016 10:40 PM, Cyril Hrubis wrote:
> Hi!
>> +int safe_prctl(const char *file, const int lineno, int opt, ...);
>> +#define SAFE_PRCTL(opt, ...) \
>> +	safe_prctl(__FILE__, __LINE__, (opt), ##__VA_ARGS__)
>> +
>>  #endif /* SAFE_MACROS_H__ */
>> diff --git a/lib/safe_macros.c b/lib/safe_macros.c
>> index 1c89b8c..c8c5383 100644
>> --- a/lib/safe_macros.c
>> +++ b/lib/safe_macros.c
>> @@ -807,3 +807,23 @@ int safe_system(const char *file, const int lineno, const char *cmd)
>>  
>>  	return rval;
>>  }
>> +
>> +int safe_prctl(const char *file, const int lineno, int opt, ...)
>> +{
>> +	va_list ap;
>> +	int rval;
>> +	unsigned long arg;
>> +
>> +	va_start(ap, opt);
>> +	arg = va_arg(ap, unsigned long);
>> +	va_end(ap);
> 
> This is broken by design. The prctl() can have up to four long arguments
> but here we use only the first one unconditionally.
> 
> 
> I'm not sure how we can fix this easily. Either we can count number of
> arguments passed to the SAFE_PRCTL() by hacky preprocessor counting[1]
> and pass it as a first agument.
> 
> Or we can switch on the opt to figure out the number of expected arguments,
> which is ugly as well, but we can at least start small by adding only
> opts that we use.
> 
> [1]
> https://groups.google.com/forum/?_escaped_fragment_=topic/comp.std.c/d-6Mj5Lko_s#!topic/comp.std.c/d-6Mj5Lko_s
> 

Thanks for your review and kindly suggestion.

I see the prctl()'s arguments is variable in prctl.h, just like the fcntl() and ioctl(),
so I think maybe I can do it just as fcntl() and ioctl() did?

Best Regards,
Guangwen Feng

>> +	rval = prctl(opt, arg);
>> +	if (rval < 0) {
>> +		tst_brkm(TBROK | TERRNO, NULL,
>> +			 "%s:%d prctl(%i, %lu) failed",
>> +			 file, lineno, opt, arg);
>> +	}
>> +
>> +	return rval;
>> +}
>> -- 
>> 1.8.4.2
>>
>>
>>
>>
>> -- 
>> Mailing list info: https://lists.linux.it/listinfo/ltp
> 

[LTP] [PATCH 4/5] SAFE_MACROS: Add SAFE_PRCTL()

[Cyril Hrubis]

Hi!
> I see the prctl()'s arguments is variable in prctl.h, just like the fcntl() and ioctl(),
> so I think maybe I can do it just as fcntl() and ioctl() did?

Ah, missed that. That would work for this case.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2 1/4] Cleanup && Drop redundant space in safe macros

[Guangwen Feng]

Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
---
 include/tst_safe_macros.h | 22 +++++++++++-----------
 lib/safe_macros.c         | 28 ++++++++++++++--------------
 2 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/include/tst_safe_macros.h b/include/tst_safe_macros.h
index d352d5a3..32b2556 100644
--- a/include/tst_safe_macros.h
+++ b/include/tst_safe_macros.h
@@ -146,7 +146,7 @@ static inline int safe_setpgid(const char *file, const int lineno,
 	rval = setpgid(pid, pgid);
 	if (rval) {
 		tst_brk_(file, lineno, TBROK | TERRNO,
-		         "setpgid(%i, %i) failed", pid, pgid);
+			 "setpgid(%i, %i) failed", pid, pgid);
 	}
 
 	return rval;
@@ -292,8 +292,8 @@ static inline void *safe_mmap(const char *file, const int lineno,
 	rval = mmap(addr, length, prot, flags, fd, offset);
 	if (rval == MAP_FAILED) {
 		tst_brk_(file, lineno, TBROK | TERRNO,
-			"mmap(%p,%zu,%d,%d,%d,%ld) failed",
-			addr, length, prot, flags, fd, (long) offset);
+			 "mmap(%p,%zu,%d,%d,%d,%ld) failed",
+			 addr, length, prot, flags, fd, (long) offset);
 	}
 
 	return rval;
@@ -362,7 +362,7 @@ 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);
+			 "fstat(%d,%p) failed", fd, buf);
 	}
 
 	return rval;
@@ -379,7 +379,7 @@ 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);
+			 "lstat(%s,%p) failed", path, buf);
 	}
 
 	return rval;
@@ -396,8 +396,8 @@ 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);
 	}
 
 	return rval;
@@ -414,8 +414,8 @@ 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);
 	}
 
 	return rval;
@@ -451,8 +451,8 @@ static inline sighandler_t safe_signal(const char *file, const int lineno,
 
 	if (rval == SIG_ERR) {
 		tst_brk_(file, lineno, TBROK | TERRNO,
-			"signal(%d,%p) failed",
-			signum, handler);
+			 "signal(%d,%p) failed",
+			 signum, handler);
 	}
 
 	return rval;
diff --git a/lib/safe_macros.c b/lib/safe_macros.c
index 094e5c7..d5c8753 100644
--- a/lib/safe_macros.c
+++ b/lib/safe_macros.c
@@ -369,7 +369,7 @@ int safe_link(const char *file, const int lineno,
 
 	if (rval == -1) {
 		tst_brkm(TBROK | TERRNO, cleanup_fn,
-		         "%s:%d: link(%s,%s) failed",
+			 "%s:%d: link(%s,%s) failed",
 			 file, lineno, oldpath, newpath);
 	}
 
@@ -421,7 +421,7 @@ int safe_symlink(const char *file, const int lineno,
 
 	if (rval == -1) {
 		tst_brkm(TBROK | TERRNO, cleanup_fn,
-		         "%s:%d: symlink(%s,%s) failed",
+			 "%s:%d: symlink(%s,%s) failed",
 			 file, lineno, oldpath, newpath);
 	}
 
@@ -437,7 +437,7 @@ ssize_t safe_write(const char *file, const int lineno, void (cleanup_fn) (void),
 	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);
+			 file, lineno, fildes, buf, rval);
 	}
 
 	return rval;
@@ -567,8 +567,8 @@ int safe_chown(const char *file, const int lineno, void (cleanup_fn)(void),
 
 	if (rval == -1) {
 		tst_brkm(TBROK | TERRNO, cleanup_fn,
-			"%s:%d: chown(%s,%d,%d) failed",
-			file, lineno, path, owner, group);
+			 "%s:%d: chown(%s,%d,%d) failed",
+			 file, lineno, path, owner, group);
 	}
 
 	return rval;
@@ -583,7 +583,7 @@ int safe_fchown(const char *file, const int lineno, void (cleanup_fn)(void),
 
 	if (rval == -1) {
 		tst_brkm(TBROK | TERRNO, cleanup_fn,
-		         "%s:%d: fchown(%d,%d,%d) failed",
+			 "%s:%d: fchown(%d,%d,%d) failed",
 			 file, lineno, fd, owner, group);
 	}
 
@@ -658,7 +658,7 @@ int safe_mkfifo(const char *file, const int lineno,
 
 	if (rval == -1) {
 		tst_brkm(TBROK | TERRNO, cleanup_fn,
-		         "%s:%d: mkfifo(%s, 0%o) failed",
+			 "%s:%d: mkfifo(%s, 0%o) failed",
 			 file, lineno, pathname, mode);
 	}
 
@@ -725,7 +725,7 @@ DIR* safe_opendir(const char *file, const int lineno, void (cleanup_fn)(void),
 
 	if (!rval) {
 		tst_brkm(TBROK | TERRNO, cleanup_fn,
-		         "%s:%d: opendir(%s) failed", file, lineno, name);
+			 "%s:%d: opendir(%s) failed", file, lineno, name);
 	}
 
 	return rval;
@@ -740,7 +740,7 @@ int safe_closedir(const char *file, const int lineno, void (cleanup_fn)(void),
 
 	if (rval) {
 		tst_brkm(TBROK | TERRNO, cleanup_fn,
-		         "%s:%d: closedir(%p) failed", file, lineno, dirp);
+			 "%s:%d: closedir(%p) failed", file, lineno, dirp);
 	}
 
 	return rval;
@@ -757,7 +757,7 @@ struct dirent *safe_readdir(const char *file, const int lineno, void (cleanup_fn
 
 	if (!rval && errno) {
 		tst_brkm(TBROK | TERRNO, cleanup_fn,
-		         "%s:%d: readdir(%p) failed", file, lineno, dirp);
+			 "%s:%d: readdir(%p) failed", file, lineno, dirp);
 	}
 
 	errno = err;
@@ -772,7 +772,7 @@ int safe_getpriority(const char *file, const int lineno, int which, id_t who)
 	rval = getpriority(which, who);
 	if (errno) {
 		tst_brkm(TBROK | TERRNO, NULL,
-		         "%s:%d getpriority(%i, %i) failed",
+			 "%s:%d getpriority(%i, %i) failed",
 			 file, lineno, which, who);
 	}
 
@@ -795,7 +795,7 @@ int safe_setxattr(const char *file, const int lineno, const char *path,
 		}
 
 		tst_brkm(TBROK | TERRNO, NULL, "%s:%d: setxattr() failed",
-			     file, lineno);
+			 file, lineno);
 	}
 
 	return rval;
@@ -816,7 +816,7 @@ int safe_lsetxattr(const char *file, const int lineno, const char *path,
 		}
 
 		tst_brkm(TBROK | TERRNO, NULL, "%s:%d: lsetxattr() failed",
-			     file, lineno);
+			 file, lineno);
 	}
 
 	return rval;
@@ -837,7 +837,7 @@ int safe_fsetxattr(const char *file, const int lineno, int fd, const char *name,
 		}
 
 		tst_brkm(TBROK | TERRNO, NULL, "%s:%d: fsetxattr() failed",
-			     file, lineno);
+			 file, lineno);
 	}
 
 	return rval;
-- 
1.8.4.2

[LTP] [PATCH v2 2/4] SAFE_MACROS: Add SAFE_SETPRIORITY()

[Guangwen Feng]

Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
---
 include/tst_safe_macros.h |  5 +++++
 lib/safe_macros.c         | 15 +++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/include/tst_safe_macros.h b/include/tst_safe_macros.h
index 32b2556..59973b6 100644
--- a/include/tst_safe_macros.h
+++ b/include/tst_safe_macros.h
@@ -471,6 +471,11 @@ int safe_getpriority(const char *file, const int lineno, int which, id_t who);
 #define SAFE_GETPRIORITY(which, who) \
 	safe_getpriority(__FILE__, __LINE__, (which), (who))
 
+int safe_setpriority(const char *file, const int lineno, int which, id_t who,
+		     int prio);
+#define SAFE_SETPRIORITY(which, who, prio) \
+	safe_setpriority(__FILE__, __LINE__, (which), (who), (prio))
+
 int safe_setxattr(const char *file, const int lineno, const char *path,
             const char *name, const void *value, size_t size, int flags);
 #define SAFE_SETXATTR(path, name, value, size, flags) \
diff --git a/lib/safe_macros.c b/lib/safe_macros.c
index d5c8753..65fe27c 100644
--- a/lib/safe_macros.c
+++ b/lib/safe_macros.c
@@ -780,6 +780,21 @@ int safe_getpriority(const char *file, const int lineno, int which, id_t who)
 	return rval;
 }
 
+int safe_setpriority(const char *file, const int lineno, int which, id_t who,
+		     int prio)
+{
+	int rval;
+
+	rval = setpriority(which, who, prio);
+	if (rval) {
+		tst_brkm(TBROK | TERRNO, NULL,
+			"%s:%d setpriority(%i, %i, %i) failed",
+			file, lineno, which, who, prio);
+	}
+
+	return rval;
+}
+
 int safe_setxattr(const char *file, const int lineno, const char *path,
 		  const char *name, const void *value, size_t size, int flags)
 {
-- 
1.8.4.2

[LTP] [PATCH v2 3/4] SAFE_MACROS: Add SAFE_PRCTL()

[Guangwen Feng]

Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
---
 include/tst_safe_macros.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/tst_safe_macros.h b/include/tst_safe_macros.h
index 59973b6..1044a34 100644
--- a/include/tst_safe_macros.h
+++ b/include/tst_safe_macros.h
@@ -278,6 +278,13 @@ static inline ssize_t safe_pwrite(const char *file, const int lineno,
 	            "fcntl(%i,%s,...) failed", fd, #cmd), 0 \
 	 : ret;})
 
+#define SAFE_PRCTL(opt, ...)                                \
+	({int ret = prctl(opt, ##__VA_ARGS__);              \
+	  ret < 0 ?                                         \
+	   tst_brk(TBROK | TERRNO,                          \
+	            "prctl(%s,...) failed", #opt), 0        \
+	 : ret;})
+
 /*
  * following functions are inline because the behaviour may depend on
  * -D_FILE_OFFSET_BITS=64 -DOFF_T=off64_t compile flags
-- 
1.8.4.2

[LTP] [PATCH v2 4/4] syscalls/setpriority01: Use new user id to avoid messing system environment

[Guangwen Feng]

setpriority(PRIO_USER, root_proc, prio) sets the priorities of
all the root processes to the specified value, though this is
kind of expected for the test, it impacts the system environment.

Create a new user and set it privileged, then test with the temp
user id to ensure the test safety.

Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
---
 .../kernel/syscalls/setpriority/setpriority01.c    | 96 +++++++++++++++++++---
 1 file changed, 85 insertions(+), 11 deletions(-)

diff --git a/testcases/kernel/syscalls/setpriority/setpriority01.c b/testcases/kernel/syscalls/setpriority/setpriority01.c
index e5111d7..83c9c1e 100644
--- a/testcases/kernel/syscalls/setpriority/setpriority01.c
+++ b/testcases/kernel/syscalls/setpriority/setpriority01.c
@@ -23,36 +23,50 @@
  */
 
 #include <errno.h>
+#include <pwd.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/capability.h>
+#include <sys/prctl.h>
 #include <sys/resource.h>
 #include <sys/time.h>
+#include <sys/types.h>
+#include <unistd.h>
+
 #include "tst_test.h"
+#include "linux_syscall_numbers.h"
+
+static const char *username = "ltp_setpriority01";
+static int pid, uid;
+static cap_user_header_t header;
+static cap_user_data_t data;
 
 static struct tcase {
 	int which;
+	int *who;
 } tcases[] = {
-	{PRIO_PROCESS},
-	{PRIO_PGRP},
-	{PRIO_USER}
+	{PRIO_PROCESS, &pid},
+	{PRIO_PGRP, &pid},
+	{PRIO_USER, &uid}
 };
 
-static void verify_setpriority(unsigned int n)
+static void setpriority_test(struct tcase *tc)
 {
-	struct tcase *tc = &tcases[n];
 	int new_prio, cur_prio;
 	int failflag = 0;
 
 	for (new_prio = -20; new_prio < 20; new_prio++) {
-		TEST(setpriority(tc->which, 0, new_prio));
+		TEST(setpriority(tc->which, *tc->who, new_prio));
 
 		if (TEST_RETURN != 0) {
 			tst_res(TFAIL | TTERRNO,
-				"setpriority(%d, 0, %d) failed",
-				tc->which, new_prio);
+				"setpriority(%d, %d, %d) failed",
+				tc->which, *tc->who, new_prio);
 			failflag = 1;
 			continue;
 		}
 
-		cur_prio = SAFE_GETPRIORITY(tc->which, 0);
+		cur_prio = SAFE_GETPRIORITY(tc->which, *tc->who);
 
 		if (cur_prio != new_prio) {
 			tst_res(TFAIL, "current priority(%d) and "
@@ -63,14 +77,74 @@ static void verify_setpriority(unsigned int n)
 	}
 
 	if (!failflag) {
-		tst_res(TPASS, "setpriority(%d, 0, -20..19) succeeded",
-			tc->which);
+		tst_res(TPASS, "setpriority(%d, %d, -20..19) succeeded",
+			tc->which, *tc->who);
+	}
+}
+
+static void verify_setpriority(unsigned int n)
+{
+	struct tcase *tc = &tcases[n];
+
+	pid = SAFE_FORK();
+	if (pid == 0) {
+		SAFE_PRCTL(PR_SET_KEEPCAPS, 1);
+
+		SAFE_SETUID(uid);
+		SAFE_SETPGID(0, 0);
+
+		if (tst_syscall(__NR_capset, header, data) == -1)
+			tst_brk(TBROK | TERRNO, "capset() failed");
+
+		TST_CHECKPOINT_WAKE_AND_WAIT(0);
+
+		exit(0);
 	}
+
+	TST_CHECKPOINT_WAIT(0);
+
+	setpriority_test(tc);
+
+	TST_CHECKPOINT_WAKE(0);
+
+	tst_reap_children();
+}
+
+static void setup(void)
+{
+	const char *const cmd_useradd[] = {"useradd", username, NULL};
+	struct passwd *ltpuser;
+
+	tst_run_cmd(cmd_useradd, NULL, NULL, 0);
+
+	ltpuser = SAFE_GETPWNAM(username);
+	uid = ltpuser->pw_uid;
+
+	header = SAFE_MALLOC(sizeof(cap_user_header_t));
+	data = SAFE_MALLOC(sizeof(cap_user_data_t));
+
+	header->version = _LINUX_CAPABILITY_VERSION;
+	header->pid = 0;
+
+	if (tst_syscall(__NR_capget, header, data) == -1)
+		tst_brk(TBROK | TERRNO, "capget() failed");
+}
+
+static void cleanup(void)
+{
+	const char *const cmd_userdel[] = {"userdel", "-r", username, NULL};
+
+	if (tst_run_cmd(cmd_userdel, NULL, NULL, 1))
+		tst_res(TWARN | TERRNO, "'userdel -r %s' failed", username);
 }
 
 static struct tst_test test = {
 	.tid = "setpriority01",
 	.tcnt = ARRAY_SIZE(tcases),
 	.needs_root = 1,
+	.forks_child = 1,
+	.needs_checkpoints = 1,
+	.setup = setup,
+	.cleanup = cleanup,
 	.test = verify_setpriority,
 };
-- 
1.8.4.2

[LTP] [PATCH v2 4/4] syscalls/setpriority01: Use new user id to avoid messing system environment

[Cyril Hrubis]

Hi!
> setpriority(PRIO_USER, root_proc, prio) sets the priorities of
> all the root processes to the specified value, though this is
> kind of expected for the test, it impacts the system environment.
> 
> Create a new user and set it privileged, then test with the temp
> user id to ensure the test safety.

I've dropped all the capability stuff from the patch since the call to
setpriority() is now done in the main test process that runs under the
root. The child is create just so that we have a process whoose priority
will be changed.

Pushed with that change, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 5/5] syscalls/setpriority01: Use new user id to avoid messing system environment

[Guangwen Feng]

setpriority(PRIO_USER, root_proc, prio) sets the priorities of
all the root processes to the specified value, though this is
kind of expected for the test, it impacts the system environment.

Create a new user and set it privileged, then test with the temp
user id to ensure the test safety.

Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
---
 .../kernel/syscalls/setpriority/setpriority01.c    | 68 +++++++++++++++++++++-
 1 file changed, 66 insertions(+), 2 deletions(-)

diff --git a/testcases/kernel/syscalls/setpriority/setpriority01.c b/testcases/kernel/syscalls/setpriority/setpriority01.c
index e5111d7..0e53784 100644
--- a/testcases/kernel/syscalls/setpriority/setpriority01.c
+++ b/testcases/kernel/syscalls/setpriority/setpriority01.c
@@ -23,9 +23,23 @@
  */
 
 #include <errno.h>
+#include <pwd.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/capability.h>
+#include <sys/prctl.h>
 #include <sys/resource.h>
 #include <sys/time.h>
+#include <sys/types.h>
+#include <unistd.h>
+
 #include "tst_test.h"
+#include "linux_syscall_numbers.h"
+
+static char username[32];
+static uid_t uid;
+static struct __user_cap_header_struct header;
+static struct __user_cap_data_struct data;
 
 static struct tcase {
 	int which;
@@ -35,9 +49,8 @@ static struct tcase {
 	{PRIO_USER}
 };
 
-static void verify_setpriority(unsigned int n)
+static void setpriority_test(struct tcase *tc)
 {
-	struct tcase *tc = &tcases[n];
 	int new_prio, cur_prio;
 	int failflag = 0;
 
@@ -68,9 +81,60 @@ static void verify_setpriority(unsigned int n)
 	}
 }
 
+static void verify_setpriority(unsigned int n)
+{
+	struct tcase *tc = &tcases[n];
+
+	if (!SAFE_FORK()) {
+		SAFE_PRCTL(PR_SET_KEEPCAPS, 1);
+
+		SAFE_SETUID(uid);
+		SAFE_SETPGID(0, 0);
+
+		if (tst_syscall(__NR_capset, &header, &data) == -1)
+			tst_brk(TBROK | TERRNO, "capset() failed");
+
+		setpriority_test(tc);
+
+		exit(0);
+	}
+
+	tst_reap_children();
+}
+
+static void setup(void)
+{
+	char cmd[64];
+	struct passwd *ltpuser;
+
+	sprintf(username, "setpriority01.%d", getpid());
+
+	sprintf(cmd, "useradd %s", username);
+	SAFE_SYSTEM(cmd);
+
+	ltpuser = SAFE_GETPWNAM(username);
+	uid = ltpuser->pw_uid;
+
+	header.version = _LINUX_CAPABILITY_VERSION;
+	header.pid = 0;
+	if (tst_syscall(__NR_capget, &header, &data) == -1)
+		tst_brk(TBROK | TERRNO, "capget() failed");
+}
+
+static void cleanup(void)
+{
+	char cmd[64];
+
+	sprintf(cmd, "userdel -r %s", username);
+	SAFE_SYSTEM(cmd);
+}
+
 static struct tst_test test = {
 	.tid = "setpriority01",
 	.tcnt = ARRAY_SIZE(tcases),
 	.needs_root = 1,
+	.forks_child = 1,
+	.setup = setup,
+	.cleanup = cleanup,
 	.test = verify_setpriority,
 };
-- 
1.8.4.2

[LTP] [PATCH 5/5] syscalls/setpriority01: Use new user id to avoid messing system environment

[Cyril Hrubis]

Hi!
>  #include "tst_test.h"
> +#include "linux_syscall_numbers.h"
> +
> +static char username[32];
> +static uid_t uid;
> +static struct __user_cap_header_struct header;
> +static struct __user_cap_data_struct data;

Are you sure these are the right structure prototypes to use? The double
underscore usually means "library internal, do not touch".

>  	int which;
> @@ -35,9 +49,8 @@ static struct tcase {
>  	{PRIO_USER}
>  };
>  
> -static void verify_setpriority(unsigned int n)
> +static void setpriority_test(struct tcase *tc)
>  {
> -	struct tcase *tc = &tcases[n];
>  	int new_prio, cur_prio;
>  	int failflag = 0;
>  
> @@ -68,9 +81,60 @@ static void verify_setpriority(unsigned int n)
>  	}
>  }
>  
> +static void verify_setpriority(unsigned int n)
> +{
> +	struct tcase *tc = &tcases[n];
> +
> +	if (!SAFE_FORK()) {
> +		SAFE_PRCTL(PR_SET_KEEPCAPS, 1);
> +
> +		SAFE_SETUID(uid);
> +		SAFE_SETPGID(0, 0);
> +
> +		if (tst_syscall(__NR_capset, &header, &data) == -1)
> +			tst_brk(TBROK | TERRNO, "capset() failed");
> +
> +		setpriority_test(tc);
> +
> +		exit(0);
> +	}
> +
> +	tst_reap_children();

Hmm, why do we run the actuall testcase in the child here? That
complicates the whole test far more than it should have.

We can instead just run pause() in the child and run the setpriority()
call in the parent with right pid/group/uid. Or did I miss something?

> +}
> +
> +static void setup(void)
> +{
> +	char cmd[64];
> +	struct passwd *ltpuser;
> +
> +	sprintf(username, "setpriority01.%d", getpid());
> +
> +	sprintf(cmd, "useradd %s", username);
> +	SAFE_SYSTEM(cmd);
> +
> +	ltpuser = SAFE_GETPWNAM(username);
> +	uid = ltpuser->pw_uid;
> +
> +	header.version = _LINUX_CAPABILITY_VERSION;
> +	header.pid = 0;
> +	if (tst_syscall(__NR_capget, &header, &data) == -1)
> +		tst_brk(TBROK | TERRNO, "capget() failed");
> +}
> +
> +static void cleanup(void)
> +{
> +	char cmd[64];
> +
> +	sprintf(cmd, "userdel -r %s", username);
> +	SAFE_SYSTEM(cmd);
> +}
> +
>  static struct tst_test test = {
>  	.tid = "setpriority01",
>  	.tcnt = ARRAY_SIZE(tcases),
>  	.needs_root = 1,
> +	.forks_child = 1,
> +	.setup = setup,
> +	.cleanup = cleanup,
>  	.test = verify_setpriority,
>  };
> -- 
> 1.8.4.2
> 
> 
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 5/5] syscalls/setpriority01: Use new user id to avoid messing system environment

[Guangwen Feng]

Hi!

On 12/05/2016 10:45 PM, Cyril Hrubis wrote:
> Hi!
>>  #include "tst_test.h"
>> +#include "linux_syscall_numbers.h"
>> +
>> +static char username[32];
>> +static uid_t uid;
>> +static struct __user_cap_header_struct header;
>> +static struct __user_cap_data_struct data;
> 
> Are you sure these are the right structure prototypes to use? The double
> underscore usually means "library internal, do not touch".
> 

It's my mistake, thanks for review, I will correct this.

>>  	int which;
>> @@ -35,9 +49,8 @@ static struct tcase {
>>  	{PRIO_USER}
>>  };
>>  
>> -static void verify_setpriority(unsigned int n)
>> +static void setpriority_test(struct tcase *tc)
>>  {
>> -	struct tcase *tc = &tcases[n];
>>  	int new_prio, cur_prio;
>>  	int failflag = 0;
>>  
>> @@ -68,9 +81,60 @@ static void verify_setpriority(unsigned int n)
>>  	}
>>  }
>>  
>> +static void verify_setpriority(unsigned int n)
>> +{
>> +	struct tcase *tc = &tcases[n];
>> +
>> +	if (!SAFE_FORK()) {
>> +		SAFE_PRCTL(PR_SET_KEEPCAPS, 1);
>> +
>> +		SAFE_SETUID(uid);
>> +		SAFE_SETPGID(0, 0);
>> +
>> +		if (tst_syscall(__NR_capset, &header, &data) == -1)
>> +			tst_brk(TBROK | TERRNO, "capset() failed");
>> +
>> +		setpriority_test(tc);
>> +
>> +		exit(0);
>> +	}
>> +
>> +	tst_reap_children();
> 
> Hmm, why do we run the actuall testcase in the child here? That
> complicates the whole test far more than it should have.
> 
> We can instead just run pause() in the child and run the setpriority()
> call in the parent with right pid/group/uid. Or did I miss something?
> 

Sorry, if we do the actual test in the parent, we have to do the process
synchronization, because parent needs child to be ready (prctl, setuid...)
and after every test, child's pause() needs to be waken up by parent, so
I feel it's more complicated by comparison...

Best Regards,
Guangwen Feng

>> +}
>> +
>> +static void setup(void)
>> +{
>> +	char cmd[64];
>> +	struct passwd *ltpuser;
>> +
>> +	sprintf(username, "setpriority01.%d", getpid());
>> +
>> +	sprintf(cmd, "useradd %s", username);
>> +	SAFE_SYSTEM(cmd);
>> +
>> +	ltpuser = SAFE_GETPWNAM(username);
>> +	uid = ltpuser->pw_uid;
>> +
>> +	header.version = _LINUX_CAPABILITY_VERSION;
>> +	header.pid = 0;
>> +	if (tst_syscall(__NR_capget, &header, &data) == -1)
>> +		tst_brk(TBROK | TERRNO, "capget() failed");
>> +}
>> +
>> +static void cleanup(void)
>> +{
>> +	char cmd[64];
>> +
>> +	sprintf(cmd, "userdel -r %s", username);
>> +	SAFE_SYSTEM(cmd);
>> +}
>> +
>>  static struct tst_test test = {
>>  	.tid = "setpriority01",
>>  	.tcnt = ARRAY_SIZE(tcases),
>>  	.needs_root = 1,
>> +	.forks_child = 1,
>> +	.setup = setup,
>> +	.cleanup = cleanup,
>>  	.test = verify_setpriority,
>>  };
>> -- 
>> 1.8.4.2
>>
>>
>>
>>
>> -- 
>> Mailing list info: https://lists.linux.it/listinfo/ltp
> 

[LTP] [PATCH 5/5] syscalls/setpriority01: Use new user id to avoid messing system environment

[Cyril Hrubis]

Hi!
> > Hmm, why do we run the actuall testcase in the child here? That
> > complicates the whole test far more than it should have.
> > 
> > We can instead just run pause() in the child and run the setpriority()
> > call in the parent with right pid/group/uid. Or did I miss something?
> > 
> 
> Sorry, if we do the actual test in the parent, we have to do the process
> synchronization, because parent needs child to be ready (prctl, setuid...)
> and after every test, child's pause() needs to be waken up by parent, so
> I feel it's more complicated by comparison...

Since we have the checkpoint synchronization in the test library this is
just a matter of calling TST_CHECKPOINT_WAIT() in the parent and
TST_CHECKPOINT_WAKE_AND_WAIT() in the child then finally
TST_CHECKPOINT_WAKE() in the parent.

-- 
Cyril Hrubis
chrubis@suse.cz