From: Paolo 'Blaisorblade' Giarrusso Fix some bugs left in the failure path of run_helper by the previous patch: it was missing one os_close_file(fds[1]) which is conditional. To use the goto handling model, I set the fd to -1 if it's already closed (I don't want to check if keeping one more pipe-end open is no problem). Also do some cosmethic cleanup: * "err" was what we returned even on success, so just use a neutral "ret". * use tabs, not spaces. * a little more comments. Signed-off-by: Paolo 'Blaisorblade' Giarrusso Signed-off-by: Andrew Morton --- 25-akpm/arch/um/drivers/net_user.c | 3 +- 25-akpm/arch/um/kernel/helper.c | 48 ++++++++++++++++++++----------------- 2 files changed, 29 insertions(+), 22 deletions(-) diff -puN arch/um/drivers/net_user.c~uml-finish-fixing-run_helper-failure-path arch/um/drivers/net_user.c --- 25/arch/um/drivers/net_user.c~uml-finish-fixing-run_helper-failure-path Tue Nov 30 15:20:36 2004 +++ 25-akpm/arch/um/drivers/net_user.c Tue Nov 30 15:20:36 2004 @@ -177,7 +177,8 @@ static int change_tramp(char **argv, cha os_close_file(fds[0]); os_close_file(fds[1]); - CATCH_EINTR(err = waitpid(pid, NULL, 0)); + if (pid > 0) + CATCH_EINTR(err = waitpid(pid, NULL, 0)); return(pid); } diff -puN arch/um/kernel/helper.c~uml-finish-fixing-run_helper-failure-path arch/um/kernel/helper.c --- 25/arch/um/kernel/helper.c~uml-finish-fixing-run_helper-failure-path Tue Nov 30 15:20:36 2004 +++ 25-akpm/arch/um/kernel/helper.c Tue Nov 30 15:20:36 2004 @@ -49,14 +49,14 @@ static int helper_child(void *arg) return(0); } -/* XXX The alloc_stack here breaks if this is called in the tracing thread */ - +/* Returns either the pid of the child process we run or -E* on failure. + * XXX The alloc_stack here breaks if this is called in the tracing thread */ int run_helper(void (*pre_exec)(void *), void *pre_data, char **argv, unsigned long *stack_out) { struct helper_data data; unsigned long stack, sp; - int pid, fds[2], err, n; + int pid, fds[2], ret, n; if((stack_out != NULL) && (*stack_out != 0)) stack = *stack_out; @@ -64,16 +64,16 @@ int run_helper(void (*pre_exec)(void *), if(stack == 0) return(-ENOMEM); - err = os_pipe(fds, 1, 0); - if(err < 0){ - printk("run_helper : pipe failed, err = %d\n", -err); + ret = os_pipe(fds, 1, 0); + if(ret < 0){ + printk("run_helper : pipe failed, ret = %d\n", -ret); goto out_free; } - err = os_set_exec_close(fds[1], 1); - if(err < 0){ - printk("run_helper : setting FD_CLOEXEC failed, err = %d\n", - -err); + ret = os_set_exec_close(fds[1], 1); + if(ret < 0){ + printk("run_helper : setting FD_CLOEXEC failed, ret = %d\n", + -ret); goto out_close; } @@ -85,30 +85,36 @@ int run_helper(void (*pre_exec)(void *), pid = clone(helper_child, (void *) sp, CLONE_VM | SIGCHLD, &data); if(pid < 0){ printk("run_helper : clone failed, errno = %d\n", errno); - err = -errno; + ret = -errno; goto out_close; } os_close_file(fds[1]); - n = os_read_file(fds[0], &err, sizeof(err)); + fds[1] = -1; + + /*Read the errno value from the child.*/ + n = os_read_file(fds[0], &ret, sizeof(ret)); if(n < 0){ - printk("run_helper : read on pipe failed, err = %d\n", -n); - err = n; + printk("run_helper : read on pipe failed, ret = %d\n", -n); + ret = n; os_kill_process(pid, 1); } else if(n != 0){ CATCH_EINTR(n = waitpid(pid, NULL, 0)); - pid = -errno; + ret = -errno; + } else { + ret = pid; } - err = pid; - out_close: +out_close: + if (fds[1] != -1) + os_close_file(fds[1]); os_close_file(fds[0]); - out_free: +out_free: if(stack_out == NULL) free_stack(stack, 0); - else *stack_out = stack; - return(err); + else *stack_out = stack; + return(ret); } int run_helper_thread(int (*proc)(void *), void *arg, unsigned int flags, @@ -139,7 +145,7 @@ int run_helper_thread(int (*proc)(void * "0x%x\n", status); free_stack(stack, stack_order); } - else *stack_out = stack; + else *stack_out = stack; return(pid); } _