Re: [PATCH v2 1/4] run-command: add new check_command helper

2013-04-02 Thread Johannes Sixt
Am 02.04.2013 12:31, schrieb Felipe Contreras:
> And persistent_waitpid() to recover the information from the last run.

I'm not a fan of this new API, because it looks like a workaround
for a problem that should have been solved in a cleaner way. But if
we can't avoid it, please also add a paragraph to
Documentation/technical/api-run-command.txt

> +int check_command(struct child_process *cmd)
> +{
> + int status;
> + pid_t waiting;
> +
> + if (cmd->last_status.valid)
> + return 0;
> +
> + while ((waiting = waitpid(cmd->pid, &status, WNOHANG)) < 0 && errno == 
> EINTR)
> + ; /* nothing */
> +
> + if (!waiting)
> + return 1;
> +
> + if (waiting == cmd->pid) {
> + cmd->last_status.valid = 1;
> + cmd->last_status.status = status;
> + return 0;
> + }
> +
> + if (waiting > 0)
> + die("BUG: waitpid reported a random pid?");
> +
> + return 0;
> +}
> +
>  static void prepare_run_command_v_opt(struct child_process *cmd,
> const char **argv,
> int opt)
> @@ -729,7 +770,7 @@ error:
>  int finish_async(struct async *async)
>  {
>  #ifdef NO_PTHREADS
> - return wait_or_whine(async->pid, "child process");
> + return wait_or_whine(cmd, async->pid, "child process");

This breaks the NO_PTHREADS build because cmd is undeclared. Perhaps
this on top:

diff --git a/run-command.c b/run-command.c
index a9fa779..a02ef62 100644
--- a/run-command.c
+++ b/run-command.c
@@ -230,7 +230,7 @@ static pid_t persistent_waitpid(struct child_process *cmd, 
pid_t pid, int *statu
 {
pid_t waiting;
 
-   if (cmd->last_status.valid) {
+   if (cmd && cmd->last_status.valid) {
*status = cmd->last_status.status;
return pid;
}
@@ -771,7 +771,7 @@ int start_async(struct async *async)
 int finish_async(struct async *async)
 {
 #ifdef NO_PTHREADS
-   return wait_or_whine(cmd, async->pid, "child process");
+   return wait_or_whine(NULL, async->pid, "child process");
 #else
void *ret = (void *)(intptr_t)(-1);
 

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 1/4] run-command: add new check_command helper

2013-04-02 Thread Felipe Contreras
And persistent_waitpid() to recover the information from the last run.

Signed-off-by: Felipe Contreras 
---
 run-command.c | 53 +++--
 run-command.h |  5 +
 2 files changed, 52 insertions(+), 6 deletions(-)

diff --git a/run-command.c b/run-command.c
index 07e27ff..b900c6e 100644
--- a/run-command.c
+++ b/run-command.c
@@ -226,14 +226,27 @@ static inline void set_cloexec(int fd)
fcntl(fd, F_SETFD, flags | FD_CLOEXEC);
 }
 
-static int wait_or_whine(pid_t pid, const char *argv0)
+static pid_t persistent_waitpid(struct child_process *cmd, pid_t pid, int 
*status)
+{
+   pid_t waiting;
+
+   if (cmd->last_status.valid) {
+   *status = cmd->last_status.status;
+   return pid;
+   }
+
+   while ((waiting = waitpid(pid, status, 0)) < 0 && errno == EINTR)
+   ;   /* nothing */
+   return waiting;
+}
+
+static int wait_or_whine(struct child_process *cmd, pid_t pid, const char 
*argv0)
 {
int status, code = -1;
pid_t waiting;
int failed_errno = 0;
 
-   while ((waiting = waitpid(pid, &status, 0)) < 0 && errno == EINTR)
-   ;   /* nothing */
+   waiting = persistent_waitpid(cmd, pid, &status);
 
if (waiting < 0) {
failed_errno = errno;
@@ -276,6 +289,8 @@ int start_command(struct child_process *cmd)
int failed_errno = failed_errno;
char *str;
 
+   cmd->last_status.valid = 0;
+
/*
 * In case of errors we must keep the promise to close FDs
 * that have been passed in via ->in and ->out.
@@ -437,7 +452,7 @@ fail_pipe:
 * At this point we know that fork() succeeded, but execvp()
 * failed. Errors have been reported to our stderr.
 */
-   wait_or_whine(cmd->pid, cmd->argv[0]);
+   wait_or_whine(cmd, cmd->pid, cmd->argv[0]);
failed_errno = errno;
cmd->pid = -1;
}
@@ -542,7 +557,7 @@ fail_pipe:
 
 int finish_command(struct child_process *cmd)
 {
-   return wait_or_whine(cmd->pid, cmd->argv[0]);
+   return wait_or_whine(cmd, cmd->pid, cmd->argv[0]);
 }
 
 int run_command(struct child_process *cmd)
@@ -553,6 +568,32 @@ int run_command(struct child_process *cmd)
return finish_command(cmd);
 }
 
+int check_command(struct child_process *cmd)
+{
+   int status;
+   pid_t waiting;
+
+   if (cmd->last_status.valid)
+   return 0;
+
+   while ((waiting = waitpid(cmd->pid, &status, WNOHANG)) < 0 && errno == 
EINTR)
+   ; /* nothing */
+
+   if (!waiting)
+   return 1;
+
+   if (waiting == cmd->pid) {
+   cmd->last_status.valid = 1;
+   cmd->last_status.status = status;
+   return 0;
+   }
+
+   if (waiting > 0)
+   die("BUG: waitpid reported a random pid?");
+
+   return 0;
+}
+
 static void prepare_run_command_v_opt(struct child_process *cmd,
  const char **argv,
  int opt)
@@ -729,7 +770,7 @@ error:
 int finish_async(struct async *async)
 {
 #ifdef NO_PTHREADS
-   return wait_or_whine(async->pid, "child process");
+   return wait_or_whine(cmd, async->pid, "child process");
 #else
void *ret = (void *)(intptr_t)(-1);
 
diff --git a/run-command.h b/run-command.h
index 221ce33..74a733d 100644
--- a/run-command.h
+++ b/run-command.h
@@ -39,11 +39,16 @@ struct child_process {
unsigned stdout_to_stderr:1;
unsigned use_shell:1;
unsigned clean_on_exit:1;
+   struct last_status {
+   unsigned valid:1;
+   int status;
+   } last_status;
 };
 
 int start_command(struct child_process *);
 int finish_command(struct child_process *);
 int run_command(struct child_process *);
+int check_command(struct child_process *);
 
 extern char *find_hook(const char *name);
 extern int run_hook(const char *index_file, const char *name, ...);
-- 
1.8.2

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html