[CRIU] [PATCH] page-xfer: close sockets on errror paths

[Andrew Vagin]

On Fri, Apr 12, 2013 at 08:46:56PM +0400, Pavel Emelyanov wrote:
> On 04/12/2013 04:14 PM, Andrey Vagin wrote:
> > CID 996195 (#1 of 1): Resource leak (RESOURCE_LEAK)
> > 10. leaked_handle: Handle variable ask going out of scope leaks the handle.
> > 
> > CID 996196 (#3 of 3): Resource leak (RESOURCE_LEAK)
> > 10. leaked_handle: Handle variable sk going out of scope leaks the handle.
> 
> Can we teach this soft not to warn us about not closed files on paths,
> that lead directly to program termination?
> 
> More comments inline.
> 
> > Signed-off-by: Andrey Vagin <avagin at openvz.org>
> > ---
> >  page-xfer.c | 27 +++++++++++++++++----------
> >  1 file changed, 17 insertions(+), 10 deletions(-)
> > 
> > diff --git a/page-xfer.c b/page-xfer.c
> > index b60f1ce..f9ae094 100644
> > --- a/page-xfer.c
> > +++ b/page-xfer.c
> > @@ -106,8 +106,11 @@ static int page_server_add(int sk, struct page_server_iov *pi)
> >  
> >  static int page_server_serve(int sk)
> >  {
> > +	int ret = -1;
> > +
> >  	if (pipe(cxfer.p)) {
> >  		pr_perror("Can't make pipe for xfer");
> > +		close(sk);
> >  		return -1;
> 
> goto out;
> 
> >  	}
> >  
> > @@ -115,7 +118,6 @@ static int page_server_serve(int sk)
> >  	pr_debug("Created xfer pipe size %u\n", cxfer.pipe_size);
> >  
> >  	while (1) {
> > -		int ret;
> >  		struct page_server_iov pi;
> >  
> >  		ret = read(sk, &pi, sizeof(pi));
> > @@ -124,7 +126,8 @@ static int page_server_serve(int sk)
> >  
> >  		if (ret != sizeof(pi)) {
> >  			pr_perror("Can't read pagemap from socket");
> > -			return -1;
> > +			ret = -1;
> > +			break;
> >  		}
> >  
> >  		switch (pi.cmd) {
> > @@ -138,16 +141,18 @@ static int page_server_serve(int sk)
> >  		}
> >  
> >  		if (ret)
> > -			return -1;
> > +			break;
> >  	}
> >  
> >  	pr_info("Session over\n");
> > -	return 0;
> 
> out:
> 
> > +
> > +	close(sk);
> > +	return ret;
> >  }
> >  
> >  int cr_page_server(void)
> >  {
> > -	int sk, ask;
> > +	int sk, ask = -1;
> >  	struct sockaddr_in caddr;
> >  	socklen_t clen = sizeof(caddr);
> >  
> > @@ -165,22 +170,24 @@ int cr_page_server(void)
> >  	opts.ps_addr.sin_family = AF_INET;
> >  	if (bind(sk, (struct sockaddr *)&opts.ps_addr, sizeof(opts.ps_addr))) {
> >  		pr_perror("Can't bind page server\n");
> > -		return -1;
> > +		goto out;
here ask is -1
> >  	}
> >  
> >  	if (listen(sk, 1)) {
> >  		pr_perror("Can't listen on page server socket");
> > -		return -1;
> > +		goto out;
> >  	}
> >  
> >  	ask = accept(sk, (struct sockaddr *)&caddr, &clen);
> 
> close(sk); here

close() can overwrite errno

> 
> > -	if (ask < 0) {
> > +	if (ask < 0)
> >  		pr_perror("Can't accept connection to server");
> > -		return -1;
> > -	}
> >  
> > +out:
> >  	close(sk);
> >  
> > +	if (ask < 0)
> > +		return -1;
> > +
> 
> and no double if (ask < 0) here

if bind fails, how can I understand that without this check?

> 
> >  	pr_info("Accepted connection from %s:%u\n",
> >  			inet_ntoa(caddr.sin_addr),
> >  			(int)ntohs(caddr.sin_port));
> > 
> 
>