[CRIU] Re: [PATCH] pstree: Rename @max_pid to @helper_task_pid

Cyrill Gorcunov gorcunov at openvz.org
Tue Oct 2 11:18:42 EDT 2012


On Tue, Oct 02, 2012 at 07:07:08PM +0400, Andrey Wagin wrote:
> 2012/10/2 Cyrill Gorcunov <gorcunov at openvz.org>:
> > This name is too abstract, rename it trying to
> > explain why we need it.
> >
> > Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
> > ---
> >  pstree.c |   16 +++++++++++-----
> >  1 files changed, 11 insertions(+), 5 deletions(-)
> >
> > diff --git a/pstree.c b/pstree.c
> > index 6033e7e..a464107 100644
> > --- a/pstree.c
> > +++ b/pstree.c
> > @@ -116,7 +116,13 @@ err:
> >         return ret;
> >  }
> >
> > -static int max_pid = 0;
> > +/*
> > + * This is the pid we use for helper tasks, it should
> > + * not ever intersect with any pid in process tree we're
> > + * restoring.
> > + */
> > +static int helper_task_pid = 0;
> 
> No. It is not one pid and here is one more thing.
> Look at this code:
> 
> max_pid = max((int)e->pid, max_pid);
> 
> max_pid is a maximum value of pid. Why do you want to rename max_pid?
> We can rename this value to last_pid, that it looks like in a kernel.

max_pid looks too abstract for me, I mean, since we use it for helper
tasks I guess better to add a comment about it and name it apropriately.

last_pid looks good for me as well. Look, it's per-file static variable
and it take some time to figure out why we need it at all.


More information about the CRIU mailing list