Re: [RFA] Checkpoint: wait the defunct process when delete it

[Pedro Alves <pedro@codesourcery.com>]

On Thursday 13 May 2010 08:58:49, Hui Zhu wrote:
> >> 2.  before call "inferior_call_waitpid" to waitpid the ptid.
> >> Check if ppid is a simple thread.  ppid > 1
> >> Check if ppid is the GDB.  If ppid is GDB, it will auto wait the ptid.
> >
> > What do you mean, it will auto wait the ptid?  AFAICS,
> >
> > (gdb) checkpoint
> > (gdb) checkpoint
> > (gdb) checkpoint
> > (gdb) checkpoint
> > (gdb) checkpoint
> > (gdb) restart 5
> > (gdb) delete checkpoint 0
> >
> > will still leave checkpoint 0 zombie?
> 
> This is because the parent of checkpoint is GDB.  GDB will auto wait
> the zombie, so I just leave them there let GDB hanle it.

You didn't answer the question.  Please point me at where is
this "gdb auto waiting".

I actually don't understand that rationale.  The `init' process
will "auto wait"  the checkpoint forks as well, so why bother in
the first place then?

> +  if ((!find_thread_ptid (fi->parent_ptid) && find_fork_ptid (fi->parent_ptid))
> +      || (find_thread_ptid (fi->parent_ptid) && is_stopped (fi->parent_ptid)))

This requires an explaning comment in the code.

> >> +
> >> +  ret = call_function_by_hand (getppid_fn, 0, NULL);
> >> +  if (ret == 0)
> >> +    return ppid;
> >
> > ??? can getppid really return 0 ?
> 
> This 0 is not the return value of getppid.

Oh, right.  :-)

> This is how function "checkpoint_command" use this function.  Check it
> just for code safe.  :)
>   ret = call_function_by_hand (fork_fn, 0, &ret);
>   do_cleanups (old_chain);
>   if (!ret)     /* Probably can't happen.  */
>     error (_("checkpoint: call_function_by_hand returned null."));

Well, the only return from call_function_by_hand does:

    gdb_assert (retval);
    return retval;

All other cases are handled by throwing an error.   So
"definitely can't happen"; please remove the new dead code
you're adding.  

BTW, you patch is not error/exception safe.  You didn't consider
the case of an `error' being thrown between switching the
fork and back.

Otherwise, the patch looks good.

-- 
Pedro Alves