[LTP] [PATCH 1/3] lib/safe_file_ops: print file info when file_lines_scanf parsed fail

[LTP] [PATCH 1/3] lib/safe_file_ops: print file info when file_lines_scanf parsed fail

[Yang Xu]

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
 lib/safe_file_ops.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/safe_file_ops.c b/lib/safe_file_ops.c
index b576cb97e..e8ac2fdb8 100644
--- a/lib/safe_file_ops.c
+++ b/lib/safe_file_ops.c
@@ -217,7 +217,7 @@ int file_lines_scanf(const char *file, const int lineno,
 
 	if (strict && ret != arg_count) {
 		tst_brkm(TBROK, cleanup_fn, "Expected %i conversions got %i"
-			" at %s:%d", arg_count, ret, file, lineno);
+			" in %s at %s:%d", arg_count, ret, path, file, lineno);
 		return 1;
 	}
 
-- 
2.23.0

[LTP] [PATCH 2/3] lib: Add TST_ASSERT_FILE_INT and TST_ASSERT_FILE_STR

[Yang Xu]

These functions are similar to TST_ASSERT_INT/STR, but they
are designed to get field value of proc or sys file. ie
NoNewPrivs field in the /proc/[pid]/status.

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
 include/tst_assert.h | 28 ++++++++++++++++++++++++++--
 lib/tst_assert.c     | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+), 2 deletions(-)

diff --git a/include/tst_assert.h b/include/tst_assert.h
index 04e80777c..913fff1b5 100644
--- a/include/tst_assert.h
+++ b/include/tst_assert.h
@@ -16,7 +16,20 @@
  * values in sysfs, procfs, etc.
  */
 void tst_assert_int(const char *file, const int lineno,
-                    const char *path, int val);
+		    const char *path, int val);
+
+#define TST_ASSERT_FILE_INT(path, buf, val) \
+	tst_assert_file_int(__FILE__, __LINE__, path, buf, val)
+
+/*
+ * Asserts that integer value stored in the buf field of file pointed by path
+ * equals to the value passed to this function. This is mostly useful for
+ * asserting correct field values in sysfs, procfs, etc.
+ */
+
+void tst_assert_file_int(const char *file, const int lineno,
+			 const char *path, const char *buf, int val);
+
 
 #define TST_ASSERT_STR(path, val) \
 	tst_assert_str(__FILE__, __LINE__, path, val)
@@ -27,6 +40,17 @@ void tst_assert_int(const char *file, const int lineno,
  * values in sysfs, procfs, etc.
  */
 void tst_assert_str(const char *file, const int lineno,
-                    const char *path, const char *val);
+		    const char *path, const char *val);
+
+#define TST_ASSERT_FILE_STR(path, buf, val) \
+	tst_assert_file_str(__FILE__, __LINE__, path, buf, val)
+
+/*
+ * Asserts that a string value stored in the buf field of file pointed by path
+ * equals to the value passed to this function. This is mostly useful for
+ * asserting correct field values in sysfs, procfs, etc.
+ */
+void tst_assert_file_str(const char *file, const int lineno,
+			 const char *path, const char *buf, const char *val);
 
 #endif /* TST_ASSERT_H__ */
diff --git a/lib/tst_assert.c b/lib/tst_assert.c
index 8ef3cd72e..65ee76473 100644
--- a/lib/tst_assert.c
+++ b/lib/tst_assert.c
@@ -4,6 +4,7 @@
  * Author: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
  * Copyright (c) 2020 Cyril Hrubis <chrubis@suse.cz>
  */
+#include <stdio.h>
 #define TST_NO_DEFAULT_MAIN
 #include "tst_assert.h"
 #include "tst_test.h"
@@ -22,6 +23,22 @@ void tst_assert_int(const char *file, const int lineno, const char *path, int va
 	tst_res_(file, lineno, TFAIL, "%s != %d got %d", path, val, sys_val);
 }
 
+void tst_assert_file_int(const char *file, const int lineno, const char *path, const char *buf, int val)
+{
+	int sys_val;
+	char fmt[1024];
+
+	sprintf(fmt, "%s: %%d", buf);
+	SAFE_FILE_LINES_SCANF(path, fmt, &sys_val);
+
+	if (val == sys_val) {
+		tst_res_(file, lineno, TPASS, "%s %s = %d", path, buf, sys_val);
+		return;
+	}
+
+	tst_res_(file, lineno, TFAIL, "%s %s != %d got %d", path, buf, val, sys_val);
+}
+
 void tst_assert_str(const char *file, const int lineno, const char *path, const char *val)
 {
 	char sys_val[1024];
@@ -34,3 +51,19 @@ void tst_assert_str(const char *file, const int lineno, const char *path, const
 
 	tst_res_(file, lineno, TFAIL, "%s != '%s' got '%s'", path, val, sys_val);
 }
+
+void tst_assert_file_str(const char *file, const int lineno, const char *path, const char *buf, const char *val)
+{
+	char sys_val[1024];
+	char fmt[2048];
+
+	sprintf(fmt, "%s: %%1024s", buf);
+	SAFE_FILE_LINES_SCANF(path, fmt, sys_val);
+
+	if (!strcmp(val, sys_val)) {
+		tst_res_(file, lineno, TPASS, "%s %s = '%s'", path, buf, sys_val);
+		return;
+	}
+
+	tst_res_(file, lineno, TFAIL, "%s %s != '%s' got '%s'", path, buf, val, sys_val);
+}
-- 
2.23.0

[LTP] [PATCH 2/3] lib: Add TST_ASSERT_FILE_INT and TST_ASSERT_FILE_STR

[Cyril Hrubis]

Hi!
>  #endif /* TST_ASSERT_H__ */
> diff --git a/lib/tst_assert.c b/lib/tst_assert.c
> index 8ef3cd72e..65ee76473 100644
> --- a/lib/tst_assert.c
> +++ b/lib/tst_assert.c
> @@ -4,6 +4,7 @@
>   * Author: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
>   * Copyright (c) 2020 Cyril Hrubis <chrubis@suse.cz>
>   */
> +#include <stdio.h>
>  #define TST_NO_DEFAULT_MAIN
>  #include "tst_assert.h"
>  #include "tst_test.h"
> @@ -22,6 +23,22 @@ void tst_assert_int(const char *file, const int lineno, const char *path, int va
>  	tst_res_(file, lineno, TFAIL, "%s != %d got %d", path, val, sys_val);
>  }
>  
> +void tst_assert_file_int(const char *file, const int lineno, const char *path, const char *buf, int val)
> +{
> +	int sys_val;
> +	char fmt[1024];
> +
> +	sprintf(fmt, "%s: %%d", buf);

If we want to keep the function as generic as possible we shouldn't add
the colon and space after the %s here.

There is no standard on proc files, for instance it wouldn't be possible
to parse /proc/vmstat if we hardcode the format string like that.

So I would just change this to "%s%%d" instead and pass "Foo: " instead
just "Foo" in the testcases.

Also I guess that we should call it prefix rather than buf, but that's
minor.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 2/3] lib: Add TST_ASSERT_FILE_INT and TST_ASSERT_FILE_STR

[Yang Xu]

Hi Cyril


> Hi!
>>   #endif /* TST_ASSERT_H__ */
>> diff --git a/lib/tst_assert.c b/lib/tst_assert.c
>> index 8ef3cd72e..65ee76473 100644
>> --- a/lib/tst_assert.c
>> +++ b/lib/tst_assert.c
>> @@ -4,6 +4,7 @@
>>    * Author: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
>>    * Copyright (c) 2020 Cyril Hrubis <chrubis@suse.cz>
>>    */
>> +#include <stdio.h>
>>   #define TST_NO_DEFAULT_MAIN
>>   #include "tst_assert.h"
>>   #include "tst_test.h"
>> @@ -22,6 +23,22 @@ void tst_assert_int(const char *file, const int lineno, const char *path, int va
>>   	tst_res_(file, lineno, TFAIL, "%s != %d got %d", path, val, sys_val);
>>   }
>>   
>> +void tst_assert_file_int(const char *file, const int lineno, const char *path, const char *buf, int val)
>> +{
>> +	int sys_val;
>> +	char fmt[1024];
>> +
>> +	sprintf(fmt, "%s: %%d", buf);
> 
> If we want to keep the function as generic as possible we shouldn't add
> the colon and space after the %s here.
> 
> There is no standard on proc files, for instance it wouldn't be possible
> to parse /proc/vmstat if we hardcode the format string like that.
> 
> So I would just change this to "%s%%d" instead and pass "Foo: " instead
> just "Foo" in the testcases.
Yes, good suggestion!!
> 
> Also I guess that we should call it prefix rather than buf, but that's
> minor.
Agree. I will send a v2 patch.

Best Regards
Yang Xu
> 

[LTP] [PATCH v2 1/3] lib/tst_assert: print correct file and lineno when parsing failed

[Yang Xu]

Use safe_file_scanf instead of SAFE_FILE_SCANF, so we can pass correct
file and lineno. ie prctl05.c

Before this patch, fail as below:
safe_file_ops.c:142: BROK: Failed to open FILE '/proc/self/comm1' for reading at tst_assert.c:29: ENOENT (2)

After this patch, fail as below:
safe_file_ops.c:142: BROK: Failed to open FILE '/proc/self/comm1' for reading at prctl05.c:63: ENOENT (2)

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
 lib/tst_assert.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/tst_assert.c b/lib/tst_assert.c
index 8ef3cd72e..f05aea222 100644
--- a/lib/tst_assert.c
+++ b/lib/tst_assert.c
@@ -12,7 +12,7 @@ void tst_assert_int(const char *file, const int lineno, const char *path, int va
 {
 	int sys_val;
 
-	SAFE_FILE_SCANF(path, "%d", &sys_val);
+	safe_file_scanf(file, lineno, NULL, path, "%d", &sys_val);
 
 	if (val == sys_val) {
 		tst_res_(file, lineno, TPASS, "%s = %d", path, val);
@@ -26,7 +26,7 @@ void tst_assert_str(const char *file, const int lineno, const char *path, const
 {
 	char sys_val[1024];
 
-	SAFE_FILE_SCANF(path, "%1024s", sys_val);
+	safe_file_scanf(file, lineno, NULL, path, "%1024s", sys_val);
 	if (!strcmp(val, sys_val)) {
 		tst_res_(file, lineno, TPASS, "%s = '%s'", path, val);
 		return;
-- 
2.23.0

[LTP] [PATCH v2 2/3] lib: Add TST_ASSERT_FILE_INT and TST_ASSERT_FILE_STR

[Yang Xu]

These functions are similar to TST_ASSERT_INT/STR, but they
are designed to get field value of proc or sys file. ie
NoNewPrivs field in the /proc/[pid]/status.

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
 doc/test-writing-guidelines.txt |  8 ++++++++
 include/tst_assert.h            | 28 ++++++++++++++++++++++++++--
 lib/newlib_tests/.gitignore     |  1 +
 lib/newlib_tests/test_assert.c  | 19 +++++++++++++++++++
 lib/tst_assert.c                | 33 +++++++++++++++++++++++++++++++++
 5 files changed, 87 insertions(+), 2 deletions(-)
 create mode 100644 lib/newlib_tests/test_assert.c

diff --git a/doc/test-writing-guidelines.txt b/doc/test-writing-guidelines.txt
index 3e33fd4c1..bda9bcfd5 100644
--- a/doc/test-writing-guidelines.txt
+++ b/doc/test-writing-guidelines.txt
@@ -2045,6 +2045,14 @@ terminated array of strings such as:
 },
 -------------------------------------------------------------------------------
 
+2.2.36 Assert sys or proc file value
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+Using TST_ASSERT_INT/STR(path, val) to assert that integer value or string stored in
+the prefix field of file pointed by path equals to the value passed to this function.
+
+Also having a similar api pair TST_ASSERT_FILE_INT/STR(path, prefix, val) to assert
+the field value of file.
+
 2.3 Writing a testcase in shell
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
diff --git a/include/tst_assert.h b/include/tst_assert.h
index 04e80777c..9969a8169 100644
--- a/include/tst_assert.h
+++ b/include/tst_assert.h
@@ -16,7 +16,20 @@
  * values in sysfs, procfs, etc.
  */
 void tst_assert_int(const char *file, const int lineno,
-                    const char *path, int val);
+		    const char *path, int val);
+
+#define TST_ASSERT_FILE_INT(path, prefix, val) \
+	tst_assert_file_int(__FILE__, __LINE__, path, prefix, val)
+
+/*
+ * Asserts that integer value stored in the prefix field of file pointed by path
+ * equals to the value passed to this function. This is mostly useful for
+ * asserting correct field values in sysfs, procfs, etc.
+ */
+
+void tst_assert_file_int(const char *file, const int lineno,
+			 const char *path, const char *prefix, int val);
+
 
 #define TST_ASSERT_STR(path, val) \
 	tst_assert_str(__FILE__, __LINE__, path, val)
@@ -27,6 +40,17 @@ void tst_assert_int(const char *file, const int lineno,
  * values in sysfs, procfs, etc.
  */
 void tst_assert_str(const char *file, const int lineno,
-                    const char *path, const char *val);
+		    const char *path, const char *val);
+
+#define TST_ASSERT_FILE_STR(path, prefix, val) \
+	tst_assert_file_str(__FILE__, __LINE__, path, prefix, val)
+
+/*
+ * Asserts that a string value stored in the prefix field of file pointed by path
+ * equals to the value passed to this function. This is mostly useful for
+ * asserting correct field values in sysfs, procfs, etc.
+ */
+void tst_assert_file_str(const char *file, const int lineno,
+			 const char *path, const char *prefix, const char *val);
 
 #endif /* TST_ASSERT_H__ */
diff --git a/lib/newlib_tests/.gitignore b/lib/newlib_tests/.gitignore
index dd9899927..fee8795b6 100644
--- a/lib/newlib_tests/.gitignore
+++ b/lib/newlib_tests/.gitignore
@@ -25,6 +25,7 @@ test18
 test19
 test20
 tst_expiration_timer
+test_assert
 test_timer
 test_exec
 test_exec_child
diff --git a/lib/newlib_tests/test_assert.c b/lib/newlib_tests/test_assert.c
new file mode 100644
index 000000000..092303893
--- /dev/null
+++ b/lib/newlib_tests/test_assert.c
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 Yang Xu <xuyang2018.jy@cn.fujitsu.com>
+ */
+
+#include <stdio.h>
+#include "tst_test.h"
+
+static void do_test(void)
+{
+	TST_ASSERT_INT("/proc/self/oom_score", 0);
+	TST_ASSERT_STR("/proc/self/comm", "test_assert");
+	TST_ASSERT_FILE_INT("/proc/self/io", "read_bytes:", 0);
+	TST_ASSERT_FILE_STR("/proc/self/status1", "State", "unexpected");
+}
+
+static struct tst_test test = {
+	.test_all = do_test,
+};
diff --git a/lib/tst_assert.c b/lib/tst_assert.c
index f05aea222..8418fb72d 100644
--- a/lib/tst_assert.c
+++ b/lib/tst_assert.c
@@ -4,6 +4,7 @@
  * Author: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
  * Copyright (c) 2020 Cyril Hrubis <chrubis@suse.cz>
  */
+#include <stdio.h>
 #define TST_NO_DEFAULT_MAIN
 #include "tst_assert.h"
 #include "tst_test.h"
@@ -22,6 +23,22 @@ void tst_assert_int(const char *file, const int lineno, const char *path, int va
 	tst_res_(file, lineno, TFAIL, "%s != %d got %d", path, val, sys_val);
 }
 
+void tst_assert_file_int(const char *file, const int lineno, const char *path, const char *prefix, int val)
+{
+	int sys_val;
+	char fmt[1024];
+
+	sprintf(fmt, "%s%%d", prefix);
+	file_lines_scanf(file, lineno, NULL, 1, path, fmt, &sys_val);
+
+	if (val == sys_val) {
+		tst_res_(file, lineno, TPASS, "%s %s = %d", path, prefix, sys_val);
+		return;
+	}
+
+	tst_res_(file, lineno, TFAIL, "%s %s != %d got %d", path, prefix, val, sys_val);
+}
+
 void tst_assert_str(const char *file, const int lineno, const char *path, const char *val)
 {
 	char sys_val[1024];
@@ -34,3 +51,19 @@ void tst_assert_str(const char *file, const int lineno, const char *path, const
 
 	tst_res_(file, lineno, TFAIL, "%s != '%s' got '%s'", path, val, sys_val);
 }
+
+void tst_assert_file_str(const char *file, const int lineno, const char *path, const char *prefix, const char *val)
+{
+	char sys_val[1024];
+	char fmt[2048];
+
+	sprintf(fmt, "%s: %%1024s", prefix);
+	file_lines_scanf(file, lineno, NULL, 1, path, fmt, sys_val);
+
+	if (!strcmp(val, sys_val)) {
+		tst_res_(file, lineno, TPASS, "%s %s = '%s'", path, prefix, sys_val);
+		return;
+	}
+
+	tst_res_(file, lineno, TFAIL, "%s %s != '%s' got '%s'", path, prefix, val, sys_val);
+}
-- 
2.23.0

[LTP] [PATCH v2 2/3] lib: Add TST_ASSERT_FILE_INT and TST_ASSERT_FILE_STR

[Cyril Hrubis]

Hi!
> +void tst_assert_file_int(const char *file, const int lineno, const char *path, const char *prefix, int val)
> +{
> +	int sys_val;
> +	char fmt[1024];
> +
> +	sprintf(fmt, "%s%%d", prefix);

I've changed this sprintf() to snprintf() and the same for the file_str
function and pushed, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2 3/3] syscalls/prctl: Use TST_ASSERT_FILE_INT/STR

[Yang Xu]

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
 testcases/kernel/syscalls/prctl/prctl06.c     | 20 ++++++--
 testcases/kernel/syscalls/prctl/prctl06.h     | 29 ++----------
 .../kernel/syscalls/prctl/prctl06_execve.c    | 13 ++++--
 testcases/kernel/syscalls/prctl/prctl07.c     | 46 +++++--------------
 4 files changed, 39 insertions(+), 69 deletions(-)

diff --git a/testcases/kernel/syscalls/prctl/prctl06.c b/testcases/kernel/syscalls/prctl/prctl06.c
index 0e1274a2a..b64c94e40 100644
--- a/testcases/kernel/syscalls/prctl/prctl06.c
+++ b/testcases/kernel/syscalls/prctl/prctl06.c
@@ -21,16 +21,18 @@
 
 static uid_t nobody_uid;
 static gid_t nobody_gid;
+static int proc_flag = 1;
+static char proc_sup[20] = "Yes";
 
 static void do_prctl(void)
 {
 	char ipc_env_var[1024];
-	char *const argv[] = {BIN_PATH, "After execve, parent process", NULL};
-	char *const childargv[] = {BIN_PATH, "After execve, child process", NULL};
+	char *const argv[] = {BIN_PATH, "After execve, parent process", proc_sup, NULL};
+	char *const childargv[] = {BIN_PATH, "After execve, child process", proc_sup, NULL};
 	char *const envp[] = {ipc_env_var, NULL };
 	int childpid;
 
-	check_no_new_privs(0, "parent");
+	check_no_new_privs(0, "parent", proc_flag);
 
 	TEST(prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0));
 	if (TST_RET == -1) {
@@ -46,14 +48,14 @@ static void do_prctl(void)
 
 	childpid = SAFE_FORK();
 	if (childpid == 0) {
-		check_no_new_privs(1, "After fork, child process");
+		check_no_new_privs(1, "After fork, child process", proc_flag);
 		execve(BIN_PATH, childargv, envp);
 		tst_brk(TFAIL | TTERRNO,
 			"child process failed to execute prctl_execve");
 
 	} else {
 		tst_reap_children();
-		check_no_new_privs(1, "parent process");
+		check_no_new_privs(1, "parent process", proc_flag);
 		execve(BIN_PATH, argv, envp);
 		tst_brk(TFAIL | TTERRNO,
 			"parent process failed to execute prctl_execve");
@@ -74,6 +76,7 @@ static void verify_prctl(void)
 static void setup(void)
 {
 	struct passwd *pw;
+	int field;
 
 	pw = SAFE_GETPWNAM("nobody");
 	nobody_uid = pw->pw_uid;
@@ -96,6 +99,13 @@ static void setup(void)
 
 	tst_brk(TBROK | TTERRNO,
 		"current environment doesn't permit PR_GET/SET_NO_NEW_PRIVS");
+
+	TEST(FILE_LINES_SCANF(PROC_STATUS, "NoNewPrivs:%d", &field));
+	if (TST_RET == 1) {
+		tst_res(TCONF, "%s doesn't support NoNewPrivs field", PROC_STATUS);
+		proc_flag = 0;
+		strcpy(proc_sup, "No");
+	}
 }
 
 static const char *const resfile[] = {
diff --git a/testcases/kernel/syscalls/prctl/prctl06.h b/testcases/kernel/syscalls/prctl/prctl06.h
index 7f5be20bb..227ce3006 100644
--- a/testcases/kernel/syscalls/prctl/prctl06.h
+++ b/testcases/kernel/syscalls/prctl/prctl06.h
@@ -24,30 +24,7 @@
 #define BIN_PATH           MNTPOINT"/"TESTBIN
 #define SUID_MODE          (S_ISUID|S_ISGID|S_IXUSR|S_IXGRP|S_IXOTH)
 
-void check_proc_field(int val, char *name)
-{
-	static int flag = 1;
-	int field = 0;
-
-	if (!flag)
-		return;
-
-	TEST(FILE_LINES_SCANF(PROC_STATUS, "NoNewPrivs:%d", &field));
-	if (TST_RET == 1) {
-		tst_res(TCONF,
-			"%s doesn't support NoNewPrivs field", PROC_STATUS);
-		flag = 0;
-		return;
-	}
-	if (val == field)
-		tst_res(TPASS, "%s %s NoNewPrivs field expected %d got %d",
-			name, PROC_STATUS, val, field);
-	else
-		tst_res(TFAIL, "%s %s NoNewPrivs field expected %d got %d",
-			name, PROC_STATUS, val, field);
-}
-
-void check_no_new_privs(int val, char *name)
+void check_no_new_privs(int val, char *name, int flag)
 {
 	TEST(prctl(PR_GET_NO_NEW_PRIVS, 0, 0, 0, 0));
 	if (TST_RET == val)
@@ -58,8 +35,8 @@ void check_no_new_privs(int val, char *name)
 		tst_res(TFAIL,
 			"%s prctl(PR_GET_NO_NEW_PRIVS) expected %d got %ld",
 			name, val, TST_RET);
-
-	check_proc_field(val, name);
+	if (flag)
+		TST_ASSERT_FILE_INT(PROC_STATUS, "NoNewPrivs:", val);
 }
 
 #endif
diff --git a/testcases/kernel/syscalls/prctl/prctl06_execve.c b/testcases/kernel/syscalls/prctl/prctl06_execve.c
index d1e60e6c2..cca261ac4 100644
--- a/testcases/kernel/syscalls/prctl/prctl06_execve.c
+++ b/testcases/kernel/syscalls/prctl/prctl06_execve.c
@@ -12,14 +12,19 @@
 int main(int argc, char **argv)
 {
 	struct passwd *pw;
+	int proc_flag;
 
 	pw = SAFE_GETPWNAM("nobody");
 
 	tst_reinit();
-	if (argc != 2)
-		tst_brk(TFAIL, "argc is %d, expected 2", argc);
-
-	check_no_new_privs(1, argv[1]);
+	if (argc != 3)
+		tst_brk(TFAIL, "argc is %d, expected 3", argc);
+
+	if (!strcmp(argv[2], "Yes"))
+		proc_flag = 1;
+	else
+		proc_flag = 0;
+	check_no_new_privs(1, argv[1], proc_flag);
 
 	TEST(getegid());
 	if (TST_RET == 0)
diff --git a/testcases/kernel/syscalls/prctl/prctl07.c b/testcases/kernel/syscalls/prctl/prctl07.c
index 79f7710a8..a6f23a098 100644
--- a/testcases/kernel/syscalls/prctl/prctl07.c
+++ b/testcases/kernel/syscalls/prctl/prctl07.c
@@ -34,36 +34,9 @@
 #include "tst_test.h"
 
 #define PROC_STATUS "/proc/self/status"
-
-#ifdef HAVE_SYS_CAPABILITY_H
-static void check_proc_capamb(char *message, int flag)
-{
-	int cap_num;
-	char CapAmb[20];
-
-	SAFE_FILE_LINES_SCANF(PROC_STATUS, "CapAmb:%s", CapAmb);
-	cap_num = strtol(CapAmb, NULL, 16);
-	if (flag == 2) {
-		if (cap_num == 0)
-			tst_res(TPASS,
-				"%s, %s CapAmb has been clear as %d",
-				message, PROC_STATUS, cap_num);
-		else
-			tst_res(TFAIL,
-				"%s, %s CapAmb has been clear expect 0, got %d",
-				message, PROC_STATUS, cap_num);
-		return;
-	}
-	if (cap_num == (1 << CAP_NET_BIND_SERVICE))
-		tst_res(flag ? TPASS : TFAIL,
-			"%s, CapAmb in %s has CAP_NET_BIND_SERVICE",
-			message, PROC_STATUS);
-	else
-		tst_res(flag ? TFAIL : TPASS,
-			"%s, CapAmb in %s doesn't have CAP_NET_BIND_SERVICE",
-			message, PROC_STATUS);
-}
-#endif
+#define ZERO_STRING "0000000000000000"
+/*CAP_NET_BIND_SERVICE stored in the CapAmb field of PROC_STATUS*/
+#define CAP_STRING  "0000000000000400"
 
 static inline void check_cap_raise(unsigned int cap, char *message, int fail_flag)
 {
@@ -127,7 +100,8 @@ static void verify_prctl(void)
 	cap_set_flag(caps, CAP_PERMITTED, numcaps, caplist, CAP_SET);
 	cap_set_proc(caps);
 
-	check_proc_capamb("At the beginning", 0);
+	tst_res(TINFO, "At the beginning");
+	TST_ASSERT_FILE_STR(PROC_STATUS, "CapAmb", ZERO_STRING);
 
 	cap_clear_flag(caps, CAP_INHERITABLE);
 	cap_set_proc(caps);
@@ -148,14 +122,17 @@ static void verify_prctl(void)
 	/*Even this cap has been in ambient set, raise succeeds and return 0*/
 	check_cap_raise(CAP_NET_BIND_SERVICE, "CAP_NET_BIND_SERIVCE twice", 0);
 
-	check_proc_capamb("After PR_CAP_AMBIENT_RAISE", 1);
+	tst_res(TINFO, "After PR_CAP_AMBIENT_RAISE");
+	TST_ASSERT_FILE_STR(PROC_STATUS, "CapAmb", CAP_STRING);
 
 	check_cap_is_set(CAP_NET_BIND_SERVICE, "CAP_NET_BIND_SERVICE was", 1);
 	check_cap_is_set(CAP_NET_RAW, "CAP_NET_RAW was", 0);
 	/*move a cap what was not in ambient set, it also return 0*/
 	check_cap_lower(CAP_NET_RAW, "CAP_NET_RAW(it wasn't in ambient set)");
 	check_cap_lower(CAP_NET_BIND_SERVICE, "CAP_NET_BIND_SERVICE(it was in ambient set)");
-	check_proc_capamb("After PR_CAP_AMBIENT_LORWER", 0);
+
+	tst_res(TINFO, "After PR_CAP_AMBIENT_LORWER");
+	TST_ASSERT_FILE_STR(PROC_STATUS, "CapAmb", ZERO_STRING);
 
 	prctl(PR_CAP_AMBIENT, PR_CAP_AMBIENT_RAISE, CAP_NET_BIND_SERVICE, 0, 0, 0);
 	tst_res(TINFO, "raise cap for clear");
@@ -165,7 +142,8 @@ static void verify_prctl(void)
 	else
 		tst_res(TFAIL | TERRNO, "PR_AMBIENT_CLEAR_ALL failed");
 
-	check_proc_capamb("After PR_CAP_AMBIENT_CLEAN_ALL", 2);
+	tst_res(TINFO, "After PR_CAP_AMBIENT_CLEAR_ALL");
+	TST_ASSERT_FILE_STR(PROC_STATUS, "CapAmb", ZERO_STRING);
 
 	cap_free(caps);
 #else
-- 
2.23.0

[LTP] [PATCH v2 3/3] syscalls/prctl: Use TST_ASSERT_FILE_INT/STR

[Cyril Hrubis]

Hi!
> +static char proc_sup[20] = "Yes";

I've changed the proc_sup to a pointer, there is no point in having
this as an array when we can just change the pointer instead of
strcpy().

Pushed, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2 1/3] lib/tst_assert: print correct file and lineno when parsing failed

[Cyril Hrubis]

Hi!
Good catch, pushed, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 3/3] syscalls/prctl: Use TST_ASSERT_FILE_INT/STR

[Yang Xu]

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
 testcases/kernel/syscalls/prctl/prctl06.c     | 20 ++++++--
 testcases/kernel/syscalls/prctl/prctl06.h     | 29 ++----------
 .../kernel/syscalls/prctl/prctl06_execve.c    | 13 ++++--
 testcases/kernel/syscalls/prctl/prctl07.c     | 46 +++++--------------
 4 files changed, 39 insertions(+), 69 deletions(-)

diff --git a/testcases/kernel/syscalls/prctl/prctl06.c b/testcases/kernel/syscalls/prctl/prctl06.c
index 0e1274a2a..b64c94e40 100644
--- a/testcases/kernel/syscalls/prctl/prctl06.c
+++ b/testcases/kernel/syscalls/prctl/prctl06.c
@@ -21,16 +21,18 @@
 
 static uid_t nobody_uid;
 static gid_t nobody_gid;
+static int proc_flag = 1;
+static char proc_sup[20] = "Yes";
 
 static void do_prctl(void)
 {
 	char ipc_env_var[1024];
-	char *const argv[] = {BIN_PATH, "After execve, parent process", NULL};
-	char *const childargv[] = {BIN_PATH, "After execve, child process", NULL};
+	char *const argv[] = {BIN_PATH, "After execve, parent process", proc_sup, NULL};
+	char *const childargv[] = {BIN_PATH, "After execve, child process", proc_sup, NULL};
 	char *const envp[] = {ipc_env_var, NULL };
 	int childpid;
 
-	check_no_new_privs(0, "parent");
+	check_no_new_privs(0, "parent", proc_flag);
 
 	TEST(prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0));
 	if (TST_RET == -1) {
@@ -46,14 +48,14 @@ static void do_prctl(void)
 
 	childpid = SAFE_FORK();
 	if (childpid == 0) {
-		check_no_new_privs(1, "After fork, child process");
+		check_no_new_privs(1, "After fork, child process", proc_flag);
 		execve(BIN_PATH, childargv, envp);
 		tst_brk(TFAIL | TTERRNO,
 			"child process failed to execute prctl_execve");
 
 	} else {
 		tst_reap_children();
-		check_no_new_privs(1, "parent process");
+		check_no_new_privs(1, "parent process", proc_flag);
 		execve(BIN_PATH, argv, envp);
 		tst_brk(TFAIL | TTERRNO,
 			"parent process failed to execute prctl_execve");
@@ -74,6 +76,7 @@ static void verify_prctl(void)
 static void setup(void)
 {
 	struct passwd *pw;
+	int field;
 
 	pw = SAFE_GETPWNAM("nobody");
 	nobody_uid = pw->pw_uid;
@@ -96,6 +99,13 @@ static void setup(void)
 
 	tst_brk(TBROK | TTERRNO,
 		"current environment doesn't permit PR_GET/SET_NO_NEW_PRIVS");
+
+	TEST(FILE_LINES_SCANF(PROC_STATUS, "NoNewPrivs:%d", &field));
+	if (TST_RET == 1) {
+		tst_res(TCONF, "%s doesn't support NoNewPrivs field", PROC_STATUS);
+		proc_flag = 0;
+		strcpy(proc_sup, "No");
+	}
 }
 
 static const char *const resfile[] = {
diff --git a/testcases/kernel/syscalls/prctl/prctl06.h b/testcases/kernel/syscalls/prctl/prctl06.h
index 7f5be20bb..a0c8ea05a 100644
--- a/testcases/kernel/syscalls/prctl/prctl06.h
+++ b/testcases/kernel/syscalls/prctl/prctl06.h
@@ -24,30 +24,7 @@
 #define BIN_PATH           MNTPOINT"/"TESTBIN
 #define SUID_MODE          (S_ISUID|S_ISGID|S_IXUSR|S_IXGRP|S_IXOTH)
 
-void check_proc_field(int val, char *name)
-{
-	static int flag = 1;
-	int field = 0;
-
-	if (!flag)
-		return;
-
-	TEST(FILE_LINES_SCANF(PROC_STATUS, "NoNewPrivs:%d", &field));
-	if (TST_RET == 1) {
-		tst_res(TCONF,
-			"%s doesn't support NoNewPrivs field", PROC_STATUS);
-		flag = 0;
-		return;
-	}
-	if (val == field)
-		tst_res(TPASS, "%s %s NoNewPrivs field expected %d got %d",
-			name, PROC_STATUS, val, field);
-	else
-		tst_res(TFAIL, "%s %s NoNewPrivs field expected %d got %d",
-			name, PROC_STATUS, val, field);
-}
-
-void check_no_new_privs(int val, char *name)
+void check_no_new_privs(int val, char *name, int flag)
 {
 	TEST(prctl(PR_GET_NO_NEW_PRIVS, 0, 0, 0, 0));
 	if (TST_RET == val)
@@ -58,8 +35,8 @@ void check_no_new_privs(int val, char *name)
 		tst_res(TFAIL,
 			"%s prctl(PR_GET_NO_NEW_PRIVS) expected %d got %ld",
 			name, val, TST_RET);
-
-	check_proc_field(val, name);
+	if (flag)
+		TST_ASSERT_FILE_INT(PROC_STATUS, "NoNewPrivs", val);
 }
 
 #endif
diff --git a/testcases/kernel/syscalls/prctl/prctl06_execve.c b/testcases/kernel/syscalls/prctl/prctl06_execve.c
index d1e60e6c2..cca261ac4 100644
--- a/testcases/kernel/syscalls/prctl/prctl06_execve.c
+++ b/testcases/kernel/syscalls/prctl/prctl06_execve.c
@@ -12,14 +12,19 @@
 int main(int argc, char **argv)
 {
 	struct passwd *pw;
+	int proc_flag;
 
 	pw = SAFE_GETPWNAM("nobody");
 
 	tst_reinit();
-	if (argc != 2)
-		tst_brk(TFAIL, "argc is %d, expected 2", argc);
-
-	check_no_new_privs(1, argv[1]);
+	if (argc != 3)
+		tst_brk(TFAIL, "argc is %d, expected 3", argc);
+
+	if (!strcmp(argv[2], "Yes"))
+		proc_flag = 1;
+	else
+		proc_flag = 0;
+	check_no_new_privs(1, argv[1], proc_flag);
 
 	TEST(getegid());
 	if (TST_RET == 0)
diff --git a/testcases/kernel/syscalls/prctl/prctl07.c b/testcases/kernel/syscalls/prctl/prctl07.c
index 79f7710a8..a6f23a098 100644
--- a/testcases/kernel/syscalls/prctl/prctl07.c
+++ b/testcases/kernel/syscalls/prctl/prctl07.c
@@ -34,36 +34,9 @@
 #include "tst_test.h"
 
 #define PROC_STATUS "/proc/self/status"
-
-#ifdef HAVE_SYS_CAPABILITY_H
-static void check_proc_capamb(char *message, int flag)
-{
-	int cap_num;
-	char CapAmb[20];
-
-	SAFE_FILE_LINES_SCANF(PROC_STATUS, "CapAmb:%s", CapAmb);
-	cap_num = strtol(CapAmb, NULL, 16);
-	if (flag == 2) {
-		if (cap_num == 0)
-			tst_res(TPASS,
-				"%s, %s CapAmb has been clear as %d",
-				message, PROC_STATUS, cap_num);
-		else
-			tst_res(TFAIL,
-				"%s, %s CapAmb has been clear expect 0, got %d",
-				message, PROC_STATUS, cap_num);
-		return;
-	}
-	if (cap_num == (1 << CAP_NET_BIND_SERVICE))
-		tst_res(flag ? TPASS : TFAIL,
-			"%s, CapAmb in %s has CAP_NET_BIND_SERVICE",
-			message, PROC_STATUS);
-	else
-		tst_res(flag ? TFAIL : TPASS,
-			"%s, CapAmb in %s doesn't have CAP_NET_BIND_SERVICE",
-			message, PROC_STATUS);
-}
-#endif
+#define ZERO_STRING "0000000000000000"
+/*CAP_NET_BIND_SERVICE stored in the CapAmb field of PROC_STATUS*/
+#define CAP_STRING  "0000000000000400"
 
 static inline void check_cap_raise(unsigned int cap, char *message, int fail_flag)
 {
@@ -127,7 +100,8 @@ static void verify_prctl(void)
 	cap_set_flag(caps, CAP_PERMITTED, numcaps, caplist, CAP_SET);
 	cap_set_proc(caps);
 
-	check_proc_capamb("At the beginning", 0);
+	tst_res(TINFO, "At the beginning");
+	TST_ASSERT_FILE_STR(PROC_STATUS, "CapAmb", ZERO_STRING);
 
 	cap_clear_flag(caps, CAP_INHERITABLE);
 	cap_set_proc(caps);
@@ -148,14 +122,17 @@ static void verify_prctl(void)
 	/*Even this cap has been in ambient set, raise succeeds and return 0*/
 	check_cap_raise(CAP_NET_BIND_SERVICE, "CAP_NET_BIND_SERIVCE twice", 0);
 
-	check_proc_capamb("After PR_CAP_AMBIENT_RAISE", 1);
+	tst_res(TINFO, "After PR_CAP_AMBIENT_RAISE");
+	TST_ASSERT_FILE_STR(PROC_STATUS, "CapAmb", CAP_STRING);
 
 	check_cap_is_set(CAP_NET_BIND_SERVICE, "CAP_NET_BIND_SERVICE was", 1);
 	check_cap_is_set(CAP_NET_RAW, "CAP_NET_RAW was", 0);
 	/*move a cap what was not in ambient set, it also return 0*/
 	check_cap_lower(CAP_NET_RAW, "CAP_NET_RAW(it wasn't in ambient set)");
 	check_cap_lower(CAP_NET_BIND_SERVICE, "CAP_NET_BIND_SERVICE(it was in ambient set)");
-	check_proc_capamb("After PR_CAP_AMBIENT_LORWER", 0);
+
+	tst_res(TINFO, "After PR_CAP_AMBIENT_LORWER");
+	TST_ASSERT_FILE_STR(PROC_STATUS, "CapAmb", ZERO_STRING);
 
 	prctl(PR_CAP_AMBIENT, PR_CAP_AMBIENT_RAISE, CAP_NET_BIND_SERVICE, 0, 0, 0);
 	tst_res(TINFO, "raise cap for clear");
@@ -165,7 +142,8 @@ static void verify_prctl(void)
 	else
 		tst_res(TFAIL | TERRNO, "PR_AMBIENT_CLEAR_ALL failed");
 
-	check_proc_capamb("After PR_CAP_AMBIENT_CLEAN_ALL", 2);
+	tst_res(TINFO, "After PR_CAP_AMBIENT_CLEAR_ALL");
+	TST_ASSERT_FILE_STR(PROC_STATUS, "CapAmb", ZERO_STRING);
 
 	cap_free(caps);
 #else
-- 
2.23.0

[LTP] [PATCH 1/3] lib/safe_file_ops: print file info when file_lines_scanf parsed fail

[Cyril Hrubis]

Hi!
>  	if (strict && ret != arg_count) {
>  		tst_brkm(TBROK, cleanup_fn, "Expected %i conversions got %i"
> -			" at %s:%d", arg_count, ret, file, lineno);
> +			" in %s at %s:%d", arg_count, ret, path, file, lineno);

I've changed the " in %s" to " FILKE '%s'" to match the style in the
rest of the file and pushed, good catch, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz