Skip to contentSkip to conversation

selftests: firmware: Add details in error logging

PATCH Superseded Harshal 2 participants 4 messages May 16, 2025 – Jun 2, 2025 Kselftest

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Harshal

author
#0 lore raw

PATCH 1/1 selftests: firmware: Add details in error logging

Specify details in logs of failed cases

Use die() instead of exit() when write to
sys_path fails

Signed-off-by Harshal <[email protected]>
---
 tools/testing/selftests/firmware/fw_namespace.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)
tools/testing/selftests/firmware/fw_namespace.c +9 -8
index 04757dc7e546..deff7f57b694 100644
@@ -38,10 +38,11 @@ static void trigger_fw(const char *fw_name, const char *sys_path)
 
 	fd = open(sys_path, O_WRONLY);
 	if (fd < 0)
-		die("open failed: %s\n",
+		die("open of sys_path failed: %s\n",
 		    strerror(errno));
 	if (write(fd, fw_name, strlen(fw_name)) != strlen(fw_name))
-		exit(EXIT_FAILURE);
+		die("write to sys_path failed: %s\n",
+		    strerror(errno));
 	close(fd);
 }
 

@@ -52,10 +53,10 @@ static void setup_fw(const char *fw_path)
 
 	fd = open(fw_path, O_WRONLY | O_CREAT, 0600);
 	if (fd < 0)
-		die("open failed: %s\n",
+		die("open of firmware file failed: %s\n",
 		    strerror(errno));
 	if (write(fd, fw, sizeof(fw) -1) != sizeof(fw) -1)
-		die("write failed: %s\n",
+		die("write to firmware file failed: %s\n",
 		    strerror(errno));
 	close(fd);
 }

@@ -66,7 +67,7 @@ static bool test_fw_in_ns(const char *fw_name, const char *sys_path, bool block_
 
 	if (block_fw_in_parent_ns)
 		if (mount("test", "/lib/firmware", "tmpfs", MS_RDONLY, NULL) == -1)
-			die("blocking firmware in parent ns failed\n");
+			die("blocking firmware in parent namespace failed\n");
 
 	child = fork();
 	if (child == -1) {

@@ -99,11 +100,11 @@ static bool test_fw_in_ns(const char *fw_name, const char *sys_path, bool block_
 			strerror(errno));
 	}
 	if (mount(NULL, "/", NULL, MS_SLAVE|MS_REC, NULL) == -1)
-		die("remount root in child ns failed\n");
+		die("remount root in child namespace failed\n");
 
 	if (!block_fw_in_parent_ns) {
 		if (mount("test", "/lib/firmware", "tmpfs", MS_RDONLY, NULL) == -1)
-			die("blocking firmware in child ns failed\n");
+			die("blocking firmware in child namespace failed\n");
 	} else
 		umount("/lib/firmware");
 

@@ -129,8 +130,8 @@ int main(int argc, char **argv)
 		die("error: failed to build full fw_path\n");
 
 	setup_fw(fw_path);
-
 	setvbuf(stdout, NULL, _IONBF, 0);
+
 	/* Positive case: firmware in PID1 mount namespace */
 	printf("Testing with firmware in parent namespace (assumed to be same file system as PID1)\n");
 	if (!test_fw_in_ns(fw_name, sys_path, false))
Signature · 1 line
2.43.0
maintainer
#1 lore raw
Quoted 4 lines · Harshal

Specify details in logs of failed cases

Use die() instead of exit() when write to
sys_path fails

Please explain why this change is needed?

Quoted 11 lines
Signed-off-by Harshal <[email protected]>
---
  tools/testing/selftests/firmware/fw_namespace.c | 17 +++++++++--------
  1 file changed, 9 insertions(+), 8 deletions(-)
tools/testing/selftests/firmware/fw_namespace.c +0 -0
index 04757dc7e546..deff7f57b694 100644

tools/testing/selftests/firmware/fw_namespace.c static void trigger_fw(const char *fw_name, const char *sys_path) · line 45

Quoted 10 lines
tools/testing/selftests/firmware/fw_namespace.c +3 -2
@@ -38,10 +38,11 @@ static void trigger_fw(const char *fw_name, const char *sys_path)
 
  	fd = open(sys_path, O_WRONLY);
  	if (fd < 0)
-		die("open failed: %s\n",
+		die("open of sys_path failed: %s\n",
  		    strerror(errno));
  	if (write(fd, fw_name, strlen(fw_name)) != strlen(fw_name))
-		exit(EXIT_FAILURE);
+		die("write to sys_path failed: %s\n",
+		    strerror(errno));

Hmm. Wrapper scripts key off of the EXIT_FAILURE - how does the output change with this change?

Quoted 40 lines
tools/testing/selftests/firmware/fw_namespace.c +5 -5
  	close(fd);
  }
 

@@ -52,10 +53,10 @@ static void setup_fw(const char *fw_path)
 
  	fd = open(fw_path, O_WRONLY | O_CREAT, 0600);
  	if (fd < 0)
-		die("open failed: %s\n",
+		die("open of firmware file failed: %s\n",
  		    strerror(errno));
  	if (write(fd, fw, sizeof(fw) -1) != sizeof(fw) -1)
-		die("write failed: %s\n",
+		die("write to firmware file failed: %s\n",
  		    strerror(errno));
  	close(fd);
  }

@@ -66,7 +67,7 @@ static bool test_fw_in_ns(const char *fw_name, const char *sys_path, bool block_
 
  	if (block_fw_in_parent_ns)
  		if (mount("test", "/lib/firmware", "tmpfs", MS_RDONLY, NULL) == -1)
-			die("blocking firmware in parent ns failed\n");
+			die("blocking firmware in parent namespace failed\n");
 
  	child = fork();
  	if (child == -1) {

@@ -99,11 +100,11 @@ static bool test_fw_in_ns(const char *fw_name, const char *sys_path, bool block_
  			strerror(errno));
  	}
  	if (mount(NULL, "/", NULL, MS_SLAVE|MS_REC, NULL) == -1)
-		die("remount root in child ns failed\n");
+		die("remount root in child namespace failed\n");
 
  	if (!block_fw_in_parent_ns) {
  		if (mount("test", "/lib/firmware", "tmpfs", MS_RDONLY, NULL) == -1)
-			die("blocking firmware in child ns failed\n");
+			die("blocking firmware in child namespace failed\n");
  	} else
  		umount("/lib/firmware");

tools/testing/selftests/firmware/fw_namespace.c int main(int argc, char **argv) · line 137

Quoted 9 lines
tools/testing/selftests/firmware/fw_namespace.c +1 -1
@@ -129,8 +130,8 @@ int main(int argc, char **argv)
  		die("error: failed to build full fw_path\n");
 
  	setup_fw(fw_path);
-
  	setvbuf(stdout, NULL, _IONBF, 0);
+
  	/* Positive case: firmware in PID1 mount namespace */
  	printf("Testing with firmware in parent namespace (assumed to be same file system as PID1)\n");
  	if (!test_fw_in_ns(fw_name, sys_path, false))

The rest look fine.

thanks,
-- Shuah

author
#2 lore raw
Quoted 21 lines · Shuah Khan
Quoted 4 lines · Harshal

Specify details in logs of failed cases

Use die() instead of exit() when write to
sys_path fails

Please explain why this change is needed?

Quoted 12 lines
Signed-off-by Harshal <[email protected]>
---
  tools/testing/selftests/firmware/fw_namespace.c | 17 +++++++++--------
  1 file changed, 9 insertions(+), 8 deletions(-)
tools/testing/selftests/firmware/fw_namespace.c +0 -0
index 04757dc7e546..deff7f57b694 100644
@@ -38,10 +38,11 @@ static void trigger_fw(const char *fw_name, const char *sys_path)
Quoted 12 lines

tools/testing/selftests/firmware/fw_namespace.c static void trigger_fw(const char *fw_name, const char *sys_path) · line 45

Quoted 9 lines
tools/testing/selftests/firmware/fw_namespace.c +3 -2
      fd = open(sys_path, O_WRONLY);
      if (fd < 0)
-             die("open failed: %s\n",
+             die("open of sys_path failed: %s\n",
                  strerror(errno));
      if (write(fd, fw_name, strlen(fw_name)) != strlen(fw_name))
-             exit(EXIT_FAILURE);
+             die("write to sys_path failed: %s\n",
+                 strerror(errno));

Hmm. Wrapper scripts key off of the EXIT_FAILURE - how does the output change with this change?

In the whole test file, all fail cases uses die() with the specified error log and error code, so just wanted to add a error log for this fault condition and maintain consistency.

Quoted 55 lines
Quoted 40 lines
tools/testing/selftests/firmware/fw_namespace.c +5 -5
      close(fd);
  }
 

@@ -52,10 +53,10 @@ static void setup_fw(const char *fw_path)
 
      fd = open(fw_path, O_WRONLY | O_CREAT, 0600);
      if (fd < 0)
-             die("open failed: %s\n",
+             die("open of firmware file failed: %s\n",
                  strerror(errno));
      if (write(fd, fw, sizeof(fw) -1) != sizeof(fw) -1)
-             die("write failed: %s\n",
+             die("write to firmware file failed: %s\n",
                  strerror(errno));
      close(fd);
  }

@@ -66,7 +67,7 @@ static bool test_fw_in_ns(const char *fw_name, const char *sys_path, bool block_
 
      if (block_fw_in_parent_ns)
              if (mount("test", "/lib/firmware", "tmpfs", MS_RDONLY, NULL) == -1)
-                     die("blocking firmware in parent ns failed\n");
+                     die("blocking firmware in parent namespace failed\n");
 
      child = fork();
      if (child == -1) {

@@ -99,11 +100,11 @@ static bool test_fw_in_ns(const char *fw_name, const char *sys_path, bool block_
                      strerror(errno));
      }
      if (mount(NULL, "/", NULL, MS_SLAVE|MS_REC, NULL) == -1)
-             die("remount root in child ns failed\n");
+             die("remount root in child namespace failed\n");
 
      if (!block_fw_in_parent_ns) {
              if (mount("test", "/lib/firmware", "tmpfs", MS_RDONLY, NULL) == -1)
-                     die("blocking firmware in child ns failed\n");
+                     die("blocking firmware in child namespace failed\n");
      } else
              umount("/lib/firmware");

tools/testing/selftests/firmware/fw_namespace.c int main(int argc, char **argv) · line 137

Quoted 9 lines
tools/testing/selftests/firmware/fw_namespace.c +1 -1
@@ -129,8 +130,8 @@ int main(int argc, char **argv)
              die("error: failed to build full fw_path\n");
 
      setup_fw(fw_path);
-
      setvbuf(stdout, NULL, _IONBF, 0);
+
      /* Positive case: firmware in PID1 mount namespace */
      printf("Testing with firmware in parent namespace (assumed to be same file system as PID1)\n");
      if (!test_fw_in_ns(fw_name, sys_path, false))

The rest look fine.

thanks,
-- Shuah

maintainer
#3 lore raw
Quoted 35 lines · Harshal Wadhwa
Quoted 21 lines · Shuah Khan
Quoted 4 lines · Harshal

Specify details in logs of failed cases

Use die() instead of exit() when write to
sys_path fails

Please explain why this change is needed?

Quoted 12 lines
Signed-off-by Harshal <[email protected]>
---
   tools/testing/selftests/firmware/fw_namespace.c | 17 +++++++++--------
   1 file changed, 9 insertions(+), 8 deletions(-)
tools/testing/selftests/firmware/fw_namespace.c +0 -0
index 04757dc7e546..deff7f57b694 100644
@@ -38,10 +38,11 @@ static void trigger_fw(const char *fw_name, const char *sys_path)
Quoted 12 lines

tools/testing/selftests/firmware/fw_namespace.c static void trigger_fw(const char *fw_name, const char *sys_path) · line 45

Quoted 9 lines
tools/testing/selftests/firmware/fw_namespace.c +3 -2
       fd = open(sys_path, O_WRONLY);
       if (fd < 0)
-             die("open failed: %s\n",
+             die("open of sys_path failed: %s\n",
                   strerror(errno));
       if (write(fd, fw_name, strlen(fw_name)) != strlen(fw_name))
-             exit(EXIT_FAILURE);
+             die("write to sys_path failed: %s\n",
+                 strerror(errno));

Hmm. Wrapper scripts key off of the EXIT_FAILURE - how does the output change with this change?

Quoted 3 lines

In the whole test file, all fail cases uses die() with the specified error log and error code, so just wanted to add a error log for this fault condition and maintain consistency.

You don't want to make changes to be consistent, especially when the change alters the behavior - this change changes the results.

thanks,
-- Shuah