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

Christoph Hellwig hch at infradead.org
Sun Apr 29 15:24:36 CEST 2012


On Sun, Apr 29, 2012 at 09:18:11PM +0800, Liu Yuan wrote:
> On 04/29/2012 06:38 PM, Christoph Hellwig wrote:
> 
> > Eww, this is too ugly for words.  Please make "pid" a private variable
> > inside the block it's used, and add a global sheep_pid variable to hold
> > the pid for the sheep process.
> > 
> 
> 
> Errr, it is not me to use pid as a global name. I just fixes the
> regression that happen to be brought in by your patch. Nevertheless, I'd
> like to use sheep_pid as a global name. I'll patch up a V4 for it.

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.

Having "pid" local just for the fork result, and a separate sheep_pid
variable to store the parent pid in the child, preferably including a
comment on why it's needed is way better.  I'm happy to provide a patch
for that.



More information about the sheepdog mailing list