[v2] Fixed NULL_RETURNS issues introduced by remote images code.

Submitted by Rodrigo Bruno on March 14, 2017, 11:18 p.m.

Details

Message ID 1489533510-8748-1-git-send-email-rbruno@gsd.inesc-id.pt
State New
Series "Fixed NULL_RETURNS issues introduced by remote images code."
Headers show

Commit Message

Rodrigo Bruno March 14, 2017, 11:18 p.m.
---
 criu/image.c | 15 ++++++++++-----
 criu/util.c  |  5 +++++
 2 files changed, 15 insertions(+), 5 deletions(-)

Patch hide | download patch | download mbox

diff --git a/criu/image.c b/criu/image.c
index 57d3c7b..e1bcea5 100644
--- a/criu/image.c
+++ b/criu/image.c
@@ -332,9 +332,14 @@  int do_open_remote_image(int dfd, char *path, int flags)
 	/* When using namespaces, the current dir is changed so we need to
 	 * change to previous working dir and back to correctly open the image
 	 * proxy and cache sockets. */
-	int save = dirfd(opendir("."));
+	DIR *save = opendir(".");
+	if (save == NULL) {
+		pr_perror("unable to open current working directory");
+		return -1;
+	}
+
 	if (fchdir(get_service_fd(IMG_FD_OFF)) < 0) {
-		pr_debug("fchdir to dfd failed!\n");
+		pr_perror("fchdir to dfd failed!\n");
 		return -1;
 	}
 
@@ -352,11 +357,11 @@  int do_open_remote_image(int dfd, char *path, int flags)
 		ret = write_remote_image_connection(snapshot_id, path, O_WRONLY);
 	}
 
-	if (fchdir(save) < 0) {
-		pr_debug("fchdir to save failed!\n");
+	if (fchdir(dirfd(save)) < 0) {
+		pr_perror("fchdir to save failed");
 		return -1;
 	}
-	close(save);
+	closedir(save);
 
 	return ret;
 }
diff --git a/criu/util.c b/criu/util.c
index 9fd8ba2..4156bf2 100644
--- a/criu/util.c
+++ b/criu/util.c
@@ -522,6 +522,11 @@  int copy_file(int fd_in, int fd_out, size_t bytes)
 	char *buffer = (char*) malloc(chunk);
 	ssize_t ret;
 
+	if (buffer == NULL) {
+		pr_perror("failed to allocate buffer to copy file");
+		return -1;
+	}
+
 	while (1) {
 		if (opts.remote) {
 			ret = read(fd_in, buffer, chunk);

Comments

Rodrigo Bruno March 15, 2017, 10:38 a.m.
Hi,

is there any VM image that I can use to test the patches (just as travis-ci
does) before submitting them?

cheers,
rodrigo

2017-03-15 0:27 GMT+00:00 Patchwork <criupatchwork@gmail.com>:

> == Series Details ==
>
> Series: Fixed NULL_RETURNS issues introduced by remote images code. (rev2)
> URL   : https://patchwork.criu.org/series/1320/
> State : failure
>
> == Logs ==
>
> For more details see: https://travis-ci.org/criupatc
> hwork/criu/builds/211161290
>
Pavel Emelianov March 15, 2017, 6:42 p.m.
On 03/15/2017 01:38 PM, Rodrigo Bruno wrote:
> Hi, 
> 
> is there any VM image that I can use to test the patches (just as travis-ci does) before submitting them?

You can turn on travis for your github criu fork (on github page),
then commit the changes into branch and push one on github, then go
make some coffee :) after a while the patch(es) will be validated
with the e-mail notification.

-- Pavel

> cheers,
> rodrigo
> 
> 2017-03-15 0:27 GMT+00:00 Patchwork <criupatchwork@gmail.com <mailto:criupatchwork@gmail.com>>:
> 
>     == Series Details ==
> 
>     Series: Fixed NULL_RETURNS issues introduced by remote images code. (rev2)
>     URL   : https://patchwork.criu.org/series/1320/ <https://patchwork.criu.org/series/1320/>
>     State : failure
> 
>     == Logs ==
> 
>     For more details see: https://travis-ci.org/criupatchwork/criu/builds/211161290 <https://travis-ci.org/criupatchwork/criu/builds/211161290>
> 
> 
> 
> 
> _______________________________________________
> CRIU mailing list
> CRIU@openvz.org
> https://lists.openvz.org/mailman/listinfo/criu
>
Rodrigo Bruno March 15, 2017, 10:56 p.m.
Okey, thanks, that is definitely useful!

2017-03-15 18:42 GMT+00:00 Pavel Emelyanov <xemul@virtuozzo.com>:

> On 03/15/2017 01:38 PM, Rodrigo Bruno wrote:
> > Hi,
> >
> > is there any VM image that I can use to test the patches (just as
> travis-ci does) before submitting them?
>
> You can turn on travis for your github criu fork (on github page),
> then commit the changes into branch and push one on github, then go
> make some coffee :) after a while the patch(es) will be validated
> with the e-mail notification.
>
> -- Pavel
>
> > cheers,
> > rodrigo
> >
> > 2017-03-15 0:27 GMT+00:00 Patchwork <criupatchwork@gmail.com <mailto:
> criupatchwork@gmail.com>>:
> >
> >     == Series Details ==
> >
> >     Series: Fixed NULL_RETURNS issues introduced by remote images code.
> (rev2)
> >     URL   : https://patchwork.criu.org/series/1320/ <
> https://patchwork.criu.org/series/1320/>
> >     State : failure
> >
> >     == Logs ==
> >
> >     For more details see: https://travis-ci.org/
> criupatchwork/criu/builds/211161290 <https://travis-ci.org/
> criupatchwork/criu/builds/211161290>
> >
> >
> >
> >
> > _______________________________________________
> > CRIU mailing list
> > CRIU@openvz.org
> > https://lists.openvz.org/mailman/listinfo/criu
> >
>
>
Andrey Vagin March 16, 2017, 1:45 a.m.
On Tue, Mar 14, 2017 at 11:18:30PM +0000, rodrigo-bruno wrote:
> ---
>  criu/image.c | 15 ++++++++++-----
>  criu/util.c  |  5 +++++
>  2 files changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/criu/image.c b/criu/image.c
> index 57d3c7b..e1bcea5 100644
> --- a/criu/image.c
> +++ b/criu/image.c
> @@ -332,9 +332,14 @@ int do_open_remote_image(int dfd, char *path, int flags)
>  	/* When using namespaces, the current dir is changed so we need to
>  	 * change to previous working dir and back to correctly open the image
>  	 * proxy and cache sockets. */
> -	int save = dirfd(opendir("."));
> +	DIR *save = opendir(".");

you use only a file descriptor, so I think you can use open()

	int old_cwd;

	old_cwd = open(".", O_PATH);
	if (old_cwd < ) {
		...


> +	if (save == NULL) {
> +		pr_perror("unable to open current working directory");
> +		return -1;
> +	}
> +
>  	if (fchdir(get_service_fd(IMG_FD_OFF)) < 0) {
> -		pr_debug("fchdir to dfd failed!\n");
> +		pr_perror("fchdir to dfd failed!\n");
>  		return -1;
>  	}
>  
> @@ -352,11 +357,11 @@ int do_open_remote_image(int dfd, char *path, int flags)
>  		ret = write_remote_image_connection(snapshot_id, path, O_WRONLY);
>  	}
>  
> -	if (fchdir(save) < 0) {
> -		pr_debug("fchdir to save failed!\n");
> +	if (fchdir(dirfd(save)) < 0) {
> +		pr_perror("fchdir to save failed");
>  		return -1;
>  	}
> -	close(save);
> +	closedir(save);
>  
>  	return ret;
>  }
> diff --git a/criu/util.c b/criu/util.c
> index 9fd8ba2..4156bf2 100644
> --- a/criu/util.c
> +++ b/criu/util.c
> @@ -522,6 +522,11 @@ int copy_file(int fd_in, int fd_out, size_t bytes)
>  	char *buffer = (char*) malloc(chunk);
>  	ssize_t ret;
>  
> +	if (buffer == NULL) {
> +		pr_perror("failed to allocate buffer to copy file");
> +		return -1;
> +	}
> +
>  	while (1) {
>  		if (opts.remote) {
>  			ret = read(fd_in, buffer, chunk);
> -- 
> 2.1.4
> 
> _______________________________________________
> CRIU mailing list
> CRIU@openvz.org
> https://lists.openvz.org/mailman/listinfo/criu
Rodrigo Bruno March 18, 2017, 5:52 p.m.
You are correct, don't know why I didn't use just open to start with. A new
version is on the way.

2017-03-16 1:45 GMT+00:00 Andrei Vagin <avagin@virtuozzo.com>:

> On Tue, Mar 14, 2017 at 11:18:30PM +0000, rodrigo-bruno wrote:
> > ---
> >  criu/image.c | 15 ++++++++++-----
> >  criu/util.c  |  5 +++++
> >  2 files changed, 15 insertions(+), 5 deletions(-)
> >
> > diff --git a/criu/image.c b/criu/image.c
> > index 57d3c7b..e1bcea5 100644
> > --- a/criu/image.c
> > +++ b/criu/image.c
> > @@ -332,9 +332,14 @@ int do_open_remote_image(int dfd, char *path, int
> flags)
> >       /* When using namespaces, the current dir is changed so we need to
> >        * change to previous working dir and back to correctly open the
> image
> >        * proxy and cache sockets. */
> > -     int save = dirfd(opendir("."));
> > +     DIR *save = opendir(".");
>
> you use only a file descriptor, so I think you can use open()
>
>         int old_cwd;
>
>         old_cwd = open(".", O_PATH);
>         if (old_cwd < ) {
>                 ...
>
>
> > +     if (save == NULL) {
> > +             pr_perror("unable to open current working directory");
> > +             return -1;
> > +     }
> > +
> >       if (fchdir(get_service_fd(IMG_FD_OFF)) < 0) {
> > -             pr_debug("fchdir to dfd failed!\n");
> > +             pr_perror("fchdir to dfd failed!\n");
> >               return -1;
> >       }
> >
> > @@ -352,11 +357,11 @@ int do_open_remote_image(int dfd, char *path, int
> flags)
> >               ret = write_remote_image_connection(snapshot_id, path,
> O_WRONLY);
> >       }
> >
> > -     if (fchdir(save) < 0) {
> > -             pr_debug("fchdir to save failed!\n");
> > +     if (fchdir(dirfd(save)) < 0) {
> > +             pr_perror("fchdir to save failed");
> >               return -1;
> >       }
> > -     close(save);
> > +     closedir(save);
> >
> >       return ret;
> >  }
> > diff --git a/criu/util.c b/criu/util.c
> > index 9fd8ba2..4156bf2 100644
> > --- a/criu/util.c
> > +++ b/criu/util.c
> > @@ -522,6 +522,11 @@ int copy_file(int fd_in, int fd_out, size_t bytes)
> >       char *buffer = (char*) malloc(chunk);
> >       ssize_t ret;
> >
> > +     if (buffer == NULL) {
> > +             pr_perror("failed to allocate buffer to copy file");
> > +             return -1;
> > +     }
> > +
> >       while (1) {
> >               if (opts.remote) {
> >                       ret = read(fd_in, buffer, chunk);
> > --
> > 2.1.4
> >
> > _______________________________________________
> > CRIU mailing list
> > CRIU@openvz.org
> > https://lists.openvz.org/mailman/listinfo/criu
>