[PATCH v3 1/9] tools subcmd: Add non-waitpid check_if_command_finished()

Ian Rogers posted 9 patches 1 month, 1 week ago
There is a newer version of this series
[PATCH v3 1/9] tools subcmd: Add non-waitpid check_if_command_finished()
Posted by Ian Rogers 1 month, 1 week ago
Using waitpid can cause stdout/stderr of the child process to be
lost. Use Linux's /prod/<pid>/status file to determine if the process
has reached the zombie state. Use the 'status' file rather than 'stat'
to avoid issues around skipping the process name.

Tested-by: James Clark <james.clark@linaro.org>
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/lib/subcmd/run-command.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/tools/lib/subcmd/run-command.c b/tools/lib/subcmd/run-command.c
index 4e3a557a2f37..0a764c25c384 100644
--- a/tools/lib/subcmd/run-command.c
+++ b/tools/lib/subcmd/run-command.c
@@ -2,6 +2,7 @@
 #include <unistd.h>
 #include <sys/types.h>
 #include <sys/stat.h>
+#include <ctype.h>
 #include <fcntl.h>
 #include <string.h>
 #include <linux/string.h>
@@ -217,8 +218,40 @@ static int wait_or_whine(struct child_process *cmd, bool block)
 
 int check_if_command_finished(struct child_process *cmd)
 {
+#ifdef __linux__
+	char filename[FILENAME_MAX + 12];
+	char status_line[256];
+	FILE *status_file;
+
+	/*
+	 * Check by reading /proc/<pid>/status as calling waitpid causes
+	 * stdout/stderr to be closed and data lost.
+	 */
+	sprintf(filename, "/proc/%d/status", cmd->pid);
+	status_file = fopen(filename, "r");
+	if (status_file == NULL) {
+		/* Open failed assume finish_command was called. */
+		return true;
+	}
+	while (fgets(status_line, sizeof(status_line), status_file) != NULL) {
+		char *p;
+
+		if (strncmp(status_line, "State:", 6))
+			continue;
+
+		fclose(status_file);
+		p = status_line + 6;
+		while (isspace(*p))
+			p++;
+		return *p == 'Z' ? 1 : 0;
+	}
+	/* Read failed assume finish_command was called. */
+	fclose(status_file);
+	return 1;
+#else
 	wait_or_whine(cmd, /*block=*/false);
 	return cmd->finished;
+#endif
 }
 
 int finish_command(struct child_process *cmd)
-- 
2.47.0.105.g07ac214952-goog
Re: [PATCH v3 1/9] tools subcmd: Add non-waitpid check_if_command_finished()
Posted by Namhyung Kim 1 month ago
On Thu, Oct 17, 2024 at 10:47:11PM -0700, Ian Rogers wrote:
> Using waitpid can cause stdout/stderr of the child process to be
> lost. Use Linux's /prod/<pid>/status file to determine if the process
> has reached the zombie state. Use the 'status' file rather than 'stat'
> to avoid issues around skipping the process name.
> 
> Tested-by: James Clark <james.clark@linaro.org>
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/lib/subcmd/run-command.c | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/tools/lib/subcmd/run-command.c b/tools/lib/subcmd/run-command.c
> index 4e3a557a2f37..0a764c25c384 100644
> --- a/tools/lib/subcmd/run-command.c
> +++ b/tools/lib/subcmd/run-command.c
> @@ -2,6 +2,7 @@
>  #include <unistd.h>
>  #include <sys/types.h>
>  #include <sys/stat.h>
> +#include <ctype.h>
>  #include <fcntl.h>
>  #include <string.h>
>  #include <linux/string.h>
> @@ -217,8 +218,40 @@ static int wait_or_whine(struct child_process *cmd, bool block)
>  
>  int check_if_command_finished(struct child_process *cmd)
>  {
> +#ifdef __linux__

Well.. I still don't like this part.  But I can live with that as I want
the improvements in the test much more. :)

Thanks,
Namhyung


> +	char filename[FILENAME_MAX + 12];
> +	char status_line[256];
> +	FILE *status_file;
> +
> +	/*
> +	 * Check by reading /proc/<pid>/status as calling waitpid causes
> +	 * stdout/stderr to be closed and data lost.
> +	 */
> +	sprintf(filename, "/proc/%d/status", cmd->pid);
> +	status_file = fopen(filename, "r");
> +	if (status_file == NULL) {
> +		/* Open failed assume finish_command was called. */
> +		return true;
> +	}
> +	while (fgets(status_line, sizeof(status_line), status_file) != NULL) {
> +		char *p;
> +
> +		if (strncmp(status_line, "State:", 6))
> +			continue;
> +
> +		fclose(status_file);
> +		p = status_line + 6;
> +		while (isspace(*p))
> +			p++;
> +		return *p == 'Z' ? 1 : 0;
> +	}
> +	/* Read failed assume finish_command was called. */
> +	fclose(status_file);
> +	return 1;
> +#else
>  	wait_or_whine(cmd, /*block=*/false);
>  	return cmd->finished;
> +#endif
>  }
>  
>  int finish_command(struct child_process *cmd)
> -- 
> 2.47.0.105.g07ac214952-goog
>