Commit 06256155 authored by Paolo \'Blaisorblade\' Giarrusso's avatar Paolo \'Blaisorblade\' Giarrusso Committed by Linus Torvalds

[PATCH] uml: finish fixing run_helper failure path

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: default avatarPaolo 'Blaisorblade' Giarrusso <blaisorblade_spam@yahoo.it>
Signed-off-by: default avatarAndrew Morton <akpm@osdl.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@osdl.org>
parent a41a3b76
...@@ -177,7 +177,8 @@ static int change_tramp(char **argv, char *output, int output_len) ...@@ -177,7 +177,8 @@ static int change_tramp(char **argv, char *output, int output_len)
os_close_file(fds[0]); os_close_file(fds[0]);
os_close_file(fds[1]); os_close_file(fds[1]);
CATCH_EINTR(err = waitpid(pid, NULL, 0)); if (pid > 0)
CATCH_EINTR(err = waitpid(pid, NULL, 0));
return(pid); return(pid);
} }
......
...@@ -49,14 +49,14 @@ static int helper_child(void *arg) ...@@ -49,14 +49,14 @@ static int helper_child(void *arg)
return(0); 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, int run_helper(void (*pre_exec)(void *), void *pre_data, char **argv,
unsigned long *stack_out) unsigned long *stack_out)
{ {
struct helper_data data; struct helper_data data;
unsigned long stack, sp; unsigned long stack, sp;
int pid, fds[2], err, n; int pid, fds[2], ret, n;
if((stack_out != NULL) && (*stack_out != 0)) if((stack_out != NULL) && (*stack_out != 0))
stack = *stack_out; stack = *stack_out;
...@@ -64,16 +64,16 @@ int run_helper(void (*pre_exec)(void *), void *pre_data, char **argv, ...@@ -64,16 +64,16 @@ int run_helper(void (*pre_exec)(void *), void *pre_data, char **argv,
if(stack == 0) if(stack == 0)
return(-ENOMEM); return(-ENOMEM);
err = os_pipe(fds, 1, 0); ret = os_pipe(fds, 1, 0);
if(err < 0){ if(ret < 0){
printk("run_helper : pipe failed, err = %d\n", -err); printk("run_helper : pipe failed, ret = %d\n", -ret);
goto out_free; goto out_free;
} }
err = os_set_exec_close(fds[1], 1); ret = os_set_exec_close(fds[1], 1);
if(err < 0){ if(ret < 0){
printk("run_helper : setting FD_CLOEXEC failed, err = %d\n", printk("run_helper : setting FD_CLOEXEC failed, ret = %d\n",
-err); -ret);
goto out_close; goto out_close;
} }
...@@ -85,30 +85,36 @@ int run_helper(void (*pre_exec)(void *), void *pre_data, char **argv, ...@@ -85,30 +85,36 @@ int run_helper(void (*pre_exec)(void *), void *pre_data, char **argv,
pid = clone(helper_child, (void *) sp, CLONE_VM | SIGCHLD, &data); pid = clone(helper_child, (void *) sp, CLONE_VM | SIGCHLD, &data);
if(pid < 0){ if(pid < 0){
printk("run_helper : clone failed, errno = %d\n", errno); printk("run_helper : clone failed, errno = %d\n", errno);
err = -errno; ret = -errno;
goto out_close; goto out_close;
} }
os_close_file(fds[1]); 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){ if(n < 0){
printk("run_helper : read on pipe failed, err = %d\n", -n); printk("run_helper : read on pipe failed, ret = %d\n", -n);
err = n; ret = n;
os_kill_process(pid, 1); os_kill_process(pid, 1);
} }
else if(n != 0){ else if(n != 0){
CATCH_EINTR(n = waitpid(pid, NULL, 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]); os_close_file(fds[0]);
out_free: out_free:
if(stack_out == NULL) if(stack_out == NULL)
free_stack(stack, 0); free_stack(stack, 0);
else *stack_out = stack; else *stack_out = stack;
return(err); return(ret);
} }
int run_helper_thread(int (*proc)(void *), void *arg, unsigned int flags, int run_helper_thread(int (*proc)(void *), void *arg, unsigned int flags,
...@@ -139,7 +145,7 @@ int run_helper_thread(int (*proc)(void *), void *arg, unsigned int flags, ...@@ -139,7 +145,7 @@ int run_helper_thread(int (*proc)(void *), void *arg, unsigned int flags,
"0x%x\n", status); "0x%x\n", status);
free_stack(stack, stack_order); free_stack(stack, stack_order);
} }
else *stack_out = stack; else *stack_out = stack;
return(pid); return(pid);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment