[lxc-devel] [PATCH] lxc-copy: cleanup

[Serge Hallyn]

Quoting Christian Brauner (christian.brauner at mailbox.org):
> Signed-off-by: Christian Brauner <christian.brauner at mailbox.org>

So on this free_mnts() thing.  If you (as this patch makes it look
like you were risking) run it twice, you'll presumably crash.  The
callers always pass in the same global variables.  So I think that
(a) free_mnts() should take no arguments and just use the globals,
and then (b) free_mnts() should set mnt_table to NULL and
mnt_table_size = 0.

Does that make sense or am I misreading something?

> ---
>  src/lxc/lxc_copy.c | 44 ++++++++++++++++++--------------------------
>  1 file changed, 18 insertions(+), 26 deletions(-)
> 
> diff --git a/src/lxc/lxc_copy.c b/src/lxc/lxc_copy.c
> index 9812176..5919bf6 100644
> --- a/src/lxc/lxc_copy.c
> +++ b/src/lxc/lxc_copy.c
> @@ -240,41 +240,36 @@ static int mk_rand_ovl_dirs(struct mnts *mnts, unsigned int num, struct lxc_argu
>  	char upperdir[MAXPATHLEN];
>  	char workdir[MAXPATHLEN];
>  	unsigned int i;
> -	int ret = 0;
> -	struct mnts *m;
> +	int ret;
> +	struct mnts *m = NULL;
>  
> -	for (i = 0; i < num; i++) {
> -		m = mnts + i;
> +	for (i = 0, m = mnts; i < num; i++, m++) {
>  		if ((m->mnt_type == LXC_MNT_OVL) || (m->mnt_type == LXC_MNT_AUFS)) {
>  			ret = snprintf(upperdir, MAXPATHLEN, "%s/%s/delta#XXXXXX",
>  					arg->newpath, arg->newname);
>  			if (ret < 0 || ret >= MAXPATHLEN)
> -				goto err;
> +				return -1;
>  			if (!mkdtemp(upperdir))
> -				goto err;
> +				return -1;
>  			m->upper = strdup(upperdir);
>  			if (!m->upper)
> -				goto err;
> +				return -1;
>  		}
>  
>  		if (m->mnt_type == LXC_MNT_OVL) {
>  			ret = snprintf(workdir, MAXPATHLEN, "%s/%s/work#XXXXXX",
>  					arg->newpath, arg->newname);
>  			if (ret < 0 || ret >= MAXPATHLEN)
> -				goto err;
> +				return -1;
>  			if (!mkdtemp(workdir))
> -				goto err;
> +				return -1;
>  			m->workdir = strdup(workdir);
>  			if (!m->workdir)
> -				goto err;
> +				return -1;
>  		}
>  	}
>  
>  	return 0;
> -
> -err:
> -	free_mnts(mnt_table, mnt_table_size);
> -	return -1;
>  }
>  
>  static char *construct_path(char *path, bool as_prefix)
> @@ -343,7 +338,6 @@ static char *set_mnt_entry(struct mnts *m)
>  	return mntentry;
>  
>  err:
> -	free_mnts(mnt_table, mnt_table_size);
>  	free(mntentry);
>  	return NULL;
>  }
> @@ -378,7 +372,6 @@ static int do_clone_ephemeral(struct lxc_container *c,
>  	int ret = 0;
>  	bool bret = true;
>  	struct lxc_container *clone;
> -	struct mnts *n;
>  
>  	lxc_attach_options_t attach_options = LXC_ATTACH_OPTIONS_DEFAULT;
>  	attach_options.env_policy = LXC_ATTACH_CLEAR_ENV;
> @@ -408,13 +401,13 @@ static int do_clone_ephemeral(struct lxc_container *c,
>  		goto err;
>  
>  	/* allocate and set mount entries */
> -	for (i = 0; i < mnt_table_size; i++) {
> +	struct mnts *n = NULL;
> +	for (i = 0, n = mnt_table; i < mnt_table_size; i++, n++) {
>  		char *mntentry = NULL;
> -		n = mnt_table + i;
> -		if ((mntentry = set_mnt_entry(n))) {
> +		mntentry = set_mnt_entry(n);
> +		if (mntentry)
>  			bret = clone->set_config_item(clone, "lxc.mount.entry", mntentry);
> -			free(mntentry);
> -		}
> +		free(mntentry);
>  		if (!mntentry || !bret)
>  			goto err;
>  	}
> @@ -432,7 +425,7 @@ static int do_clone_ephemeral(struct lxc_container *c,
>  	}
>  
>  	if (!clone->start(clone, 0, NULL)) {
> -		if (!(clone->lxc_conf->ephemeral == 1))
> +		if (clone->lxc_conf->ephemeral != 1)
>  			goto err;
>  		goto put;
>  	}
> @@ -492,17 +485,16 @@ static int do_clone_task(struct lxc_container *c, enum task task, int flags,
>  static void free_mnts(struct mnts *m, unsigned int num)
>  {
>  	unsigned int i;
> -	struct mnts *n;
> +	struct mnts *n = NULL;
>  
> -	for (i = 0; i < num; i++) {
> -		n = m + i;
> +	for (i = 0, n = m; i < num; i++, n++) {
>  		free(n->src);
>  		free(n->dest);
>  		free(n->options);
>  		free(n->upper);
>  		free(n->workdir);
>  	}
> -	free(m);
> +	free(n);
>  }
>  
>  /* we pass fssize in bytes */
> -- 
> 2.7.0
> 
> _______________________________________________
> lxc-devel mailing list
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel