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 |