[Sheepdog] [PATCH v3] logger: correct crash_handler output

[Liu Yuan]

On 04/29/2012 09:24 PM, Christoph Hellwig wrote:

> Having it global before was ugly but survivable, the problem is
> overloading the meaning in your patch - in the parent it contains the
> fork() result, which is the child pid, using one and the same variable
> for the parent pid in the child is bound to cause confusion


No, my patch just fixes what you forgot to do. See your below commit:
cc5e7ee93a67acc535af3659e9664f9ef381cbf6

@@ -457,13 +461,13 @@ notrace int log_init(char *program_name,
 			exit(1);
 		}

-		pid = getppid();
-		/* flush on daemon's crash */
-		sa_new.sa_handler = (void*)log_sigexit;
-		sigemptyset(&sa_new.sa_mask);
+		/* flush when either the logger or its parent dies */
+		sa_new.sa_handler = crash_handler;
 		sa_new.sa_flags = 0;
-		sigaction(SIGSEGV, &sa_new, &sa_old );
-		sigaction(SIGHUP, &sa_new, &sa_old );
+		sigemptyset(&sa_new.sa_mask);
+
+		sigaction(SIGSEGV, &sa_new, &sa_old);
+		sigaction(SIGHUP, &sa_new, &sa_old);

 		prctl(PR_SET_PDEATHSIG, SIGHUP);

At last, I didn't intend to voice it out, since it is kind of trivial
and mean to do it. But we are talking right here, I want to remind that
it is not the first time your patch breaks the thing. Yes, it is partly
the viewer's fault to merge it too hasty, but otherwise partly you
should extend test coverage before submitting the patch.

Thanks,
Yuan