[CRIU] Re: [PATCH v2] restore: close opened fd

[Pavel Emelyanov]

On 02/20/2012 06:24 PM, Kinsbursky Stanislav wrote:
> v2: Some more cleanup around fixup_pages_data()
> 
> This is a cleanup patch.
> 
> Signed-off-by: Stanislav Kinsbursky <skinsbursky at openvz.org>
> 
> ---
>  cr-restore.c |  116 ++++++++++++++++++++++++++++------------------------------
>  1 files changed, 56 insertions(+), 60 deletions(-)
> 
> diff --git a/cr-restore.c b/cr-restore.c
> index 8208509..4443990 100644
> --- a/cr-restore.c
> +++ b/cr-restore.c
> @@ -641,7 +641,7 @@ static inline bool should_restore_page(int pid, unsigned long va)
>  
>  static int fixup_pages_data(int pid, int fd)
>  {
> -	int pgfd, ret;
> +	int pgfd = -1, shmfd = -1, ret = -1;

Why do we replace one fd with two?

>  	u64 va;
>  
>  	pr_info("%d: Reading shmem pages img\n", pid);
> @@ -651,9 +651,8 @@ static int fixup_pages_data(int pid, int fd)
>  		return -1;
>  
>  	while (1) {
> -		ret = read_img(pgfd, &va);
> -		if (ret < 0)
> -			return -1;
> +		if (read_img(pgfd, &va) < 0)
> +			goto out;
>  
>  		if (va == 0)
>  			break;
> @@ -662,35 +661,32 @@ static int fixup_pages_data(int pid, int fd)
>  		sendfile(fd, pgfd, NULL, PAGE_SIZE);
>  	}
>  
> -	close(pgfd);
> -
> -	pgfd = open_image_ro(CR_FD_PAGES_SHMEM, pid);
> -	if (pgfd < 0)
> -		return -1;
> +	shmfd = open_image_ro(CR_FD_PAGES_SHMEM, pid);
> +	if (shmfd < 0)
> +		goto out;
>  
>  	while (1) {
> -		ret = read_img(pgfd, &va);
> -		if (ret < 0)
> -			return -1;
> +		if (read_img(shmfd, &va) < 0)
> +			goto out;
>  
>  		if (va == 0)
>  			break;
>  
>  		if (!should_restore_page(pid, va)) {
> -			lseek(pgfd, PAGE_SIZE, SEEK_CUR);
> +			lseek(shmfd, PAGE_SIZE, SEEK_CUR);
>  			continue;
>  		}
>  
>  		pr_info("%d: Restoring shared page: %16lx\n", pid, va);
>  		write(fd, &va, sizeof(va));
> -		sendfile(fd, pgfd, NULL, PAGE_SIZE);
> +		sendfile(fd, shmfd, NULL, PAGE_SIZE);
>  	}
>  
> -	close(pgfd);
> -
> -	write_img(fd, &zero_page_entry);
> -
> -	return 0;
> +	ret = write_img(fd, &zero_page_entry);
> +out:
> +	close_safe(&pgfd);
> +	close_safe(&shmfd);
> +	return ret;
>  }
>  
>  static int prepare_image_maps(int fd, int pid)