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

[Pavel Emelyanov]

On 03/04/2012 01:39 AM, Cyrill Gorcunov wrote:
> And clean it up a bit as well.
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
> ---
>  cr-restore.c |   15 ++++++++-------
>  1 files changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/cr-restore.c b/cr-restore.c
> index 720f23f..46295c3 100644
> --- a/cr-restore.c
> +++ b/cr-restore.c
> @@ -1232,9 +1232,9 @@ struct cr_clone_arg {
>  
>  static inline int fork_with_pid(int pid, unsigned long ns_clone_flags)
>  {
> +	struct cr_clone_arg ca = { };

What for?

>  	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.

>  	if (ca.fd < 0) {
>  		pr_perror("%d: Can't open %s", pid, LAST_PID_PATH);
>  		goto err;
> @@ -1257,9 +1256,10 @@ static inline int fork_with_pid(int pid, unsigned long ns_clone_flags)
>  
>  	if (flock(ca.fd, LOCK_EX)) {
>  		pr_perror("%d: Can't lock %s", pid, LAST_PID_PATH);
> -		goto err;
> +		goto err_close;
>  	}
>  
> +	snprintf(buf, sizeof(buf), "%d", pid - 1);
>  	if (write_img_buf(ca.fd, buf, strlen(buf)))
>  		goto err_unlock;
>  
> @@ -1273,11 +1273,12 @@ err_unlock:
>  	if (flock(ca.fd, LOCK_UN))
>  		pr_perror("%d: Can't unlock %s", pid, LAST_PID_PATH);
>  
> +err_close:
> +	close_safe(&ca.fd);
>  err:
>  	if (stack != MAP_FAILED)
>  		munmap(stack, STACK_SIZE);
>  
> -	close_safe(&ca.fd);
>  	return ret;
>  }
>