[CRIU] Re: [PATCH 2/3] restore: Don't close LAST_PID_PATH descriptor if it was not opened

Cyrill Gorcunov gorcunov at openvz.org
Mon Mar 5 10:41:38 EST 2012


On Mon, Mar 05, 2012 at 07:29:38PM +0400, Pavel Emelyanov wrote:
> >  
> >  static inline int fork_with_pid(int pid, unsigned long ns_clone_flags)
> >  {
> > +	struct cr_clone_arg ca = { };
> 
> What for?
> 

This is a good practice to zeroify structures on stack id they
are not inited via (struct){...} somewhere else or passed into
function which initizlize it. I think I've overdone here, I'll
drop this snippet and update the patch.

> >  	int ret = -1;
> >  	char buf[32];
> > -	struct cr_clone_arg ca;
> >  	void *stack;
> >  
> >  	pr_info("Forking task with %d pid (flags %lx)\n", pid, ns_clone_flags);
> > @@ -1246,10 +1246,9 @@ static inline int fork_with_pid(int pid, unsigned long ns_clone_flags)
> >  		goto err;
> >  	}
> >  
> > -	snprintf(buf, sizeof(buf), "%d", pid - 1);
> > -	ca.pid = pid;
> > -	ca.clone_flags = ns_clone_flags;
> > -	ca.fd = open(LAST_PID_PATH, O_RDWR);
> > +	ca.pid		= pid;
> > +	ca.clone_flags	= ns_clone_flags;
> > +	ca.fd		= open(LAST_PID_PATH, O_RDWR);
> 
> In general I don't appreciate such style of assignments.

Sure I'll not toss this lines in new version, but in general
I think this style of assignment a way more convenient to
read and parse by eyes. So if you don't mind I'll continue
using it (if there is strong disagreement -- then sure I'll
stop using this style).

	Cyrill


More information about the CRIU mailing list