]> granicus.if.org Git - strace/commitdiff
Replace suspicious popen_pid assignment with an obviously correct one
authorDenys Vlasenko <dvlasenk@redhat.com>
Tue, 2 Jul 2013 09:31:24 +0000 (11:31 +0200)
committerDenys Vlasenko <dvlasenk@redhat.com>
Tue, 2 Jul 2013 09:31:24 +0000 (11:31 +0200)
popen_pid = vfork() does work correctly, but for a subtle reason
that wrong assignment of 0 happens in the child _first_,
and _then_ correct value overwrites it in the parent.

(And in a hyphothetical system where vfork = fork,
popen_pid wouldn't be shared, so it will also be ok.)

However, it's not necessary to be difficult.
This change makes it so that assignment is done only in parent.

Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
strace.c

index 4c70b988ea743e23f98923772b22aadf4e7a3e20..27412013c959d882346c0aaff79fd51e105991fe 100644 (file)
--- a/strace.c
+++ b/strace.c
@@ -485,6 +485,7 @@ static FILE *
 strace_popen(const char *command)
 {
        FILE *fp;
+       int pid;
        int fds[2];
 
        swap_uid();
@@ -493,11 +494,11 @@ strace_popen(const char *command)
 
        set_cloexec_flag(fds[1]); /* never fails */
 
-       popen_pid = vfork();
-       if (popen_pid == -1)
+       pid = vfork();
+       if (pid < 0)
                perror_msg_and_die("vfork");
 
-       if (popen_pid == 0) {
+       if (pid == 0) {
                /* child */
                close(fds[1]);
                if (fds[0] != 0) {
@@ -510,6 +511,7 @@ strace_popen(const char *command)
        }
 
        /* parent */
+       popen_pid = pid;
        close(fds[0]);
        swap_uid();
        fp = fdopen(fds[1], "w");