[lxc-devel] [PATCH 2/2] lxc_start: free the conf if starting the container fails

[Serge Hallyn]

Quoting Weng Meiling (wengmeiling.weng at huawei.com):
> 
> When running lxc-start command with valgrind, it reports a memory leak error.
> When lxc-start command fails, the conf which is from malloc has not been released.
> This patch fix the problem.
> 
> Signed-off-by: Weng Meiling <wengmeiling.weng at huawei.com>

This isn't really a problem since lxc_start will immediately exit and
free that memory.

The patch does look ok.  I'm a bit torn, as cleaning up when we don't need
to can make the code harder to read (it doesn't yet here, but will in
other places), but on the other hand false positives in valgrind could
drown out more serious issues...

I guess I'll

Acked-by: Serge E. Hallyn <serge.hallyn at ubuntu.com>

but with no blind promise to ack other patches which do un-needed frees.

> ---
>  src/lxc/lxc_start.c |   21 +++++++++++----------
>  1 files changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/src/lxc/lxc_start.c b/src/lxc/lxc_start.c
> index 957fdb0..014c37b 100644
> --- a/src/lxc/lxc_start.c
> +++ b/src/lxc/lxc_start.c
> @@ -196,25 +196,25 @@ int main(int argc, char *argv[])
> 
>  	if (rcfile && lxc_config_read(rcfile, conf)) {
>  		ERROR("failed to read configuration file");
> -		return err;
> +		goto out;
>  	}
> 
>  	if (lxc_config_define_load(&defines, conf))
> -		return err;
> +		goto out;
> 
>  	if (!rcfile && !strcmp("/sbin/init", args[0])) {
>  		ERROR("no configuration file for '/sbin/init' (may crash the host)");
> -		return err;
> +		goto out;
>  	}
> 
>  	if (ensure_path(&conf->console.path, my_args.console) < 0) {
>  		ERROR("failed to ensure console path '%s'", my_args.console);
> -		return err;
> +		goto out;
>  	}
> 
>  	if (ensure_path(&conf->console.log_path, my_args.console_log) < 0) {
>  		ERROR("failed to ensure console log '%s'", my_args.console_log);
> -		return err;
> +		goto out;
>  	}
> 
>  	if (my_args.pidfile != NULL) {
> @@ -222,7 +222,7 @@ int main(int argc, char *argv[])
>  		if (pid_fp == NULL) {
>  			SYSERROR("failed to create pidfile '%s' for '%s'",
>  				 my_args.pidfile, my_args.name);
> -			return err;
> +			goto out;
>  		}
>  	}
> 
> @@ -232,19 +232,19 @@ int main(int argc, char *argv[])
> 
>  		if (!lxc_caps_check()) {
>  			ERROR("Not running with sufficient privilege");
> -			return err;
> +			goto out;
>  		}
> 
>  		if (daemon(0, 0)) {
>  			SYSERROR("failed to daemonize '%s'", my_args.name);
> -			return err;
> +			goto out;
>  		}
>  	}
> 
>  	if (pid_fp != NULL) {
>  		if (fprintf(pid_fp, "%d\n", getpid()) < 0) {
>  			SYSERROR("failed to write '%s'", my_args.pidfile);
> -			return err;
> +			goto out;
>  		}
>  		fclose(pid_fp);
>  	}
> @@ -267,7 +267,8 @@ int main(int argc, char *argv[])
> 
>  	if (my_args.pidfile)
>  		unlink(my_args.pidfile);
> -
> +out:
> +	lxc_conf_free(conf);
>  	return err;
>  }
> 
> -- 1.7.1
> 
> 
> ------------------------------------------------------------------------------
> Get 100% visibility into Java/.NET code with AppDynamics Lite
> It's a free troubleshooting tool designed for production
> Get down to code-level detail for bottlenecks, with <2% overhead.
> Download for free and get started troubleshooting in minutes.
> http://p.sf.net/sfu/appdyn_d2d_ap2
> _______________________________________________
> Lxc-devel mailing list
> Lxc-devel at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/lxc-devel