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

[Serge Hallyn]

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)

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
>