[LTP] [PATCH 0/2] Fix proc parsing in newuname01

[LTP] [PATCH 0/2] Fix proc parsing in newuname01

[Cyril Hrubis]

Turns out some of the strings in procfs can be empty (file with just a
newline) and in such case scanf("%s", ...) fails to read the file while
what we needed instead is a function that would produce an empty string.
This patchset addds such function and makes use of it in the
newuname01 test.

Cyril Hrubis (2):
  lib: safe_file_ops: Add SAFE_FILE_READ_STR()
  syscalls: newuname01: Fix fail on empty domainname

 include/tst_safe_file_ops.h                   | 21 ++++++++++++++++
 lib/safe_file_ops.c                           | 24 +++++++++++++++++++
 .../kernel/syscalls/newuname/newuname01.c     |  8 +++----
 3 files changed, 49 insertions(+), 4 deletions(-)

-- 
2.52.0


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

[LTP] [PATCH 1/2] lib: safe_file_ops: Add SAFE_FILE_READ_STR()

[Cyril Hrubis]

Unlike the scanf("%s") this function can properly handle empty files and
in case of an empty file produces an empty string.

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 include/tst_safe_file_ops.h | 21 +++++++++++++++++++++
 lib/safe_file_ops.c         | 24 ++++++++++++++++++++++++
 2 files changed, 45 insertions(+)

diff --git a/include/tst_safe_file_ops.h b/include/tst_safe_file_ops.h
index 0d8819594..d9110ba9c 100644
--- a/include/tst_safe_file_ops.h
+++ b/include/tst_safe_file_ops.h
@@ -14,6 +14,27 @@
 	safe_file_scanf(__FILE__, __LINE__, NULL, \
 	                (path), (fmt), ## __VA_ARGS__)
 
+/**
+ * SAFE_FILE_READ_STR() - Reads a string from a file.
+ *
+ * Unlike scanf("%s") this function works fine with empty files or files that
+ * consist only of white spaces. In such case an empty string is stored into
+ * the supplied buffer.
+ *
+ * It's recomended to use this for various sysfs or procfs files that may be
+ * empty.
+ *
+ * @path: A path to a file.
+ * @buf: A buffer to store the string into.
+ * @buf_size: A buffer size.
+ */
+#define SAFE_FILE_READ_STR(path, buf, buf_size) \
+	safe_file_read_str(__FILE__, __LINE__, \
+	                   (path), (buf), (buf_size))
+
+void safe_file_read_str(const char *file, const int lineno,
+                        const char *path, char *buf, size_t buf_size);
+
 #define FILE_LINES_SCANF(path, fmt, ...) \
 	file_lines_scanf(__FILE__, __LINE__, NULL, 0,\
 			(path), (fmt), ## __VA_ARGS__)
diff --git a/lib/safe_file_ops.c b/lib/safe_file_ops.c
index 8314c4b1b..2d163c0af 100644
--- a/lib/safe_file_ops.c
+++ b/lib/safe_file_ops.c
@@ -160,6 +160,30 @@ void safe_file_scanf(const char *file, const int lineno,
 	}
 }
 
+void safe_file_read_str(const char *file, const int lineno,
+                        const char *path, char *buf, size_t buf_size)
+{
+	FILE *f;
+
+	f = fopen(path, "r");
+	if (!f) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+		          "Failed to open FILE '%s' for reading", path);
+	}
+
+	if (!fgets(buf, buf_size, f))
+		buf[0] = 0;
+
+	size_t len = strlen(buf);
+
+	if (len && buf[len-1] == '\n')
+		buf[len-1] = 0;
+
+	if (fclose(f)) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
+			"Failed to close FILE '%s'", path);
+	}
+}
 
 /*
  * Try to parse each line from file specified by 'path' according
-- 
2.52.0


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

Re: [LTP] [PATCH 1/2] lib: safe_file_ops: Add SAFE_FILE_READ_STR()

[Cyril Hrubis]

Hi!
> diff --git a/lib/safe_file_ops.c b/lib/safe_file_ops.c
> index 8314c4b1b..2d163c0af 100644
> --- a/lib/safe_file_ops.c
> +++ b/lib/safe_file_ops.c
> @@ -160,6 +160,30 @@ void safe_file_scanf(const char *file, const int lineno,
>  	}
>  }
>  
> +void safe_file_read_str(const char *file, const int lineno,
> +                        const char *path, char *buf, size_t buf_size)
> +{
> +	FILE *f;
> +
> +	f = fopen(path, "r");
> +	if (!f) {
> +		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
> +		          "Failed to open FILE '%s' for reading", path);

We need a return; here in an unlikely case that someone would call this
from a test cleanup() callback.

> +	}
> +
> +	if (!fgets(buf, buf_size, f))
> +		buf[0] = 0;
> +
> +	size_t len = strlen(buf);
> +
> +	if (len && buf[len-1] == '\n')
> +		buf[len-1] = 0;
> +
> +	if (fclose(f)) {
> +		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
> +			"Failed to close FILE '%s'", path);
> +	}
> +}
>  
>  /*
>   * Try to parse each line from file specified by 'path' according
> -- 
> 2.52.0
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

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

[LTP] [PATCH 2/2] syscalls: newuname01: Fix fail on empty domainname

[Cyril Hrubis]

The domainname can be empty hence we cannot use scanf() string for
format for reading the file. Use the newly added SAFE_FILE_READ_STR()
to read the strings from procfs instead.

Fixes: https://github.com/linux-test-project/ltp/issues/1304
Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 testcases/kernel/syscalls/newuname/newuname01.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/testcases/kernel/syscalls/newuname/newuname01.c b/testcases/kernel/syscalls/newuname/newuname01.c
index 37058023a..995ea978e 100644
--- a/testcases/kernel/syscalls/newuname/newuname01.c
+++ b/testcases/kernel/syscalls/newuname/newuname01.c
@@ -31,16 +31,16 @@ static void run(void)
 
 	TST_EXP_EQ_STR(name->sysname, "Linux");
 
-	SAFE_FILE_SCANF("/proc/sys/kernel/hostname", "%1023[^\n]", proc_val);
+	SAFE_FILE_READ_STR("/proc/sys/kernel/hostname", proc_val, sizeof(proc_val));
 	TST_EXP_EQ_STR(name->nodename, proc_val);
 
-	SAFE_FILE_SCANF("/proc/sys/kernel/osrelease", "%1023[^\n]", proc_val);
+	SAFE_FILE_READ_STR("/proc/sys/kernel/osrelease", proc_val, sizeof(proc_val));
 	TST_EXP_EQ_STR(name->release, proc_val);
 
-	SAFE_FILE_SCANF("/proc/sys/kernel/version", "%1023[^\n]", proc_val);
+	SAFE_FILE_READ_STR("/proc/sys/kernel/version", proc_val, sizeof(proc_val));
 	TST_EXP_EQ_STR(name->version, proc_val);
 
-	SAFE_FILE_SCANF("/proc/sys/kernel/domainname", "%1023[^\n]", proc_val);
+	SAFE_FILE_READ_STR("/proc/sys/kernel/domainname", proc_val, sizeof(proc_val));
 	TST_EXP_EQ_STR(name->domainname, proc_val);
 }
 
-- 
2.52.0


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

Re: [LTP] [PATCH 0/2] Fix proc parsing in newuname01

[Andrea Cervesato via ltp]

Agent didn't send the following email:

--- Reply to [PATCH 1/2] ---

On Thu, 9 Apr 2026, Cyril Hrubis wrote:
> lib: safe_file_ops: Add SAFE_FILE_READ_STR()

Hi Cyril,

> +	f = fopen(path, "r");
> +	if (!f) {
> +		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
> +		          "Failed to open FILE '%s' for reading", path);
> +	}
> +
> +	if (!fgets(buf, buf_size, f))

Missing return; after tst_brkm_() — f is NULL if fopen failed, so fgets()
dereferences NULL. Every other error path in this file has return; after
tst_brkm_().

Same issue in the fclose() error path below.

> +	size_t len = strlen(buf);

Variable declaration after a statement. Move len to the top of the function
with FILE *f.

> + * It's recomended to use this for various sysfs or procfs files that may be

s/recomended/recommended/

Regards,
LTP AI Reviewer

--
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com

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

Re: [LTP] [PATCH 0/2] Fix proc parsing in newuname01

[Cyril Hrubis]

Hi!
> > +	f = fopen(path, "r");
> > +	if (!f) {
> > +		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
> > +		          "Failed to open FILE '%s' for reading", path);
> > +	}
> > +
> > +	if (!fgets(buf, buf_size, f))
> 
> Missing return; after tst_brkm_() — f is NULL if fopen failed, so fgets()
> dereferences NULL. Every other error path in this file has return; after
> tst_brkm_().

Yes I'm aware, thanks for reminding me though.

> > +	size_t len = strlen(buf);
> 
> Variable declaration after a statement. Move len to the top of the function
> with FILE *f.

Do we stil follow the C89 in this regard? We already force gnu99 in
the config.mk.

> > + * It's recomended to use this for various sysfs or procfs files that may be
> 
> s/recomended/recommended/
> 
> Regards,
> LTP AI Reviewer
> 
> --
> Andrea Cervesato
> SUSE QE Automation Engineer Linux
> andrea.cervesato@suse.com

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH 0/2] Fix proc parsing in newuname01

[Andrea Cervesato via ltp]

Hi Cyril,

> Hi!
> > > +	f = fopen(path, "r");
> > > +	if (!f) {
> > > +		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
> > > +		          "Failed to open FILE '%s' for reading", path);
> > > +	}
> > > +
> > > +	if (!fgets(buf, buf_size, f))
> > 
> > Missing return; after tst_brkm_() — f is NULL if fopen failed, so fgets()
> > dereferences NULL. Every other error path in this file has return; after
> > tst_brkm_().
> 
> Yes I'm aware, thanks for reminding me though.
> 
> > > +	size_t len = strlen(buf);
> > 
> > Variable declaration after a statement. Move len to the top of the function
> > with FILE *f.
> 
> Do we stil follow the C89 in this regard? We already force gnu99 in
> the config.mk.

It doesn't make any sense indeed. I added the c99 agent rule, but we told
it to follow kernel linux conventions, which are specific, including this
one.

We can safely ignore and see if it happens again with other reviews, and
eventually update the configuration.

Regards,
--
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com

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

Re: [LTP] [PATCH 0/2] Fix proc parsing in newuname01

[Cyril Hrubis]

Hi!
> > Do we stil follow the C89 in this regard? We already force gnu99 in
> > the config.mk.
> 
> It doesn't make any sense indeed. I added the c99 agent rule, but we told
> it to follow kernel linux conventions, which are specific, including this
> one.

I do not think that they enforce that rule these days. AFAIK kernel
forces even newer c11 standard for some time.

-- 
Cyril Hrubis
chrubis@suse.cz

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