[lxc-devel] [PATCH] lxc_monitor: fix memory leak on @fds and close fds

[Serge Hallyn]

Quoting Serge Hallyn (serge.hallyn at ubuntu.com):
> Quoting Arjun Sreedharan (arjun024 at gmail.com):
> > also label and consolidate error conditions for
> > better readability.
> > 
> > Signed-off-by: Arjun Sreedharan <arjun024 at gmail.com>
> 
> My only concern here is that rc in main is a pretty
> generic name - could you please either put a comment by its
> definition saying "this is main's return value, do not
> use it for anything else", or use a different variable
> name?  (You can add my ack when you resend)

(and of course make close_fds static as per your second posting :)

> Acked-by: Serge E. Hallyn <serge.hallyn at ubuntu.com>
> 
> > ---
> >  src/lxc/lxc_monitor.c | 50 ++++++++++++++++++++++++++++++++++++--------------
> >  1 file changed, 36 insertions(+), 14 deletions(-)
> > 
> > diff --git a/src/lxc/lxc_monitor.c b/src/lxc/lxc_monitor.c
> > index b3d8912..30b8bfe 100644
> > --- a/src/lxc/lxc_monitor.c
> > +++ b/src/lxc/lxc_monitor.c
> > @@ -71,6 +71,18 @@ Options :\n\
> >  	.lxcpath_additional = -1,
> >  };
> >  
> > +void close_fds(struct pollfd *fds, nfds_t nfds)
> > +{
> > +	nfds_t i;
> > +
> > +	if (nfds < 1)
> > +		return;
> > +
> > +	for (i = 0; i < nfds; ++i) {
> > +		close(fds[i].fd);
> > +	}
> > +}
> > +
> >  int main(int argc, char *argv[])
> >  {
> >  	char *regexp;
> > @@ -78,7 +90,9 @@ int main(int argc, char *argv[])
> >  	regex_t preg;
> >  	struct pollfd *fds;
> >  	nfds_t nfds;
> > -	int len, rc, i;
> > +	int len, rc, rc_snp, i;
> > +
> > +	rc = 0;
> >  
> >  	if (lxc_arguments_parse(&my_args, argc, argv))
> >  		return 1;
> > @@ -119,24 +133,24 @@ int main(int argc, char *argv[])
> >  		ERROR("failed to allocate memory");
> >  		return 1;
> >  	}
> > -	rc = snprintf(regexp, len, "^%s$", my_args.name);
> > -	if (rc < 0 || rc >= len) {
> > +	rc_snp = snprintf(regexp, len, "^%s$", my_args.name);
> > +	if (rc_snp < 0 || rc_snp >= len) {
> >  		ERROR("Name too long");
> > -		free(regexp);
> > -		return 1;
> > +		rc = 1;
> > +		goto error;
> >  	}
> >  
> >  	if (regcomp(&preg, regexp, REG_NOSUB|REG_EXTENDED)) {
> >  		ERROR("failed to compile the regex '%s'", my_args.name);
> > -		free(regexp);
> > -		return 1;
> > +		rc = 1;
> > +		goto error;
> >  	}
> > -	free(regexp);
> >  
> >  	fds = malloc(my_args.lxcpath_cnt * sizeof(struct pollfd));
> >  	if (!fds) {
> >  		SYSERROR("out of memory");
> > -		return -1;
> > +		rc = -1;
> > +		goto error;
> >  	}
> >  
> >  	nfds = my_args.lxcpath_cnt;
> > @@ -147,8 +161,9 @@ int main(int argc, char *argv[])
> >  
> >  		fd = lxc_monitor_open(my_args.lxcpath[i]);
> >  		if (fd < 0) {
> > -			regfree(&preg);
> > -			return 1;
> > +			close_fds(fds, i);
> > +			rc = 1;
> > +			goto cleanup;
> >  		}
> >  		fds[i].fd = fd;
> >  		fds[i].events = POLLIN;
> > @@ -159,8 +174,8 @@ int main(int argc, char *argv[])
> >  
> >  	for (;;) {
> >  		if (lxc_monitor_read_fdset(fds, nfds, &msg, -1) < 0) {
> > -			regfree(&preg);
> > -			return 1;
> > +			rc = 1;
> > +			goto close_and_clean;
> >  		}
> >  
> >  		msg.name[sizeof(msg.name)-1] = '\0';
> > @@ -182,7 +197,14 @@ int main(int argc, char *argv[])
> >  		}
> >  	}
> >  
> > +close_and_clean:
> > +	close_fds(fds, nfds);
> > +
> > +cleanup:
> >  	regfree(&preg);
> > +	free(fds);
> >  
> > -	return 0;
> > +error:
> > +	free(regexp);
> > +	return rc;
> >  }
> > -- 
> > 1.7.11.7
> > 
> _______________________________________________
> lxc-devel mailing list
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel