[lxc-devel] [PATCH] Cleanup parts of lxc-destroy

[Serge Hallyn]

Quoting Christian Brauner (christianvanbrauner at gmail.com):
> A bit of pedantry usually doesn't hurt. The code should be easier to follow now
> and avoids some repetitions.
> 
> Signed-off-by: Christian Brauner <christianvanbrauner at gmail.com>

The second hunk is ok, the first I think makes it worse, because it
becomes more work to track the !c1 case.  If you wanted to get rid
of the goto you could either 'if (!c1) { counter++; continue; }'
or put the c1->destroy..lxc_container_put() under if (c1).

> ---
>  src/lxc/lxc_destroy.c | 31 +++++++++++++++++--------------
>  1 file changed, 17 insertions(+), 14 deletions(-)
> 
> diff --git a/src/lxc/lxc_destroy.c b/src/lxc/lxc_destroy.c
> index f1830fd..ccab720 100644
> --- a/src/lxc/lxc_destroy.c
> +++ b/src/lxc/lxc_destroy.c
> @@ -141,6 +141,7 @@ static int do_destroy_with_snapshots(struct lxc_container *c)
>  {
>  	struct lxc_container *c1;
>  	struct stat fbuf;
> +	bool bret = false;
>  	char path[MAXPATHLEN];
>  	char *buf = NULL;
>  	char *lxcpath = NULL;
> @@ -184,16 +185,19 @@ static int do_destroy_with_snapshots(struct lxc_container *c)
>  			if (!(lxcname = strtok_r(NULL, "\n", &scratch)))
>  				break;
>  			c1 = lxc_container_new(lxcname, lxcpath);
> -			if (!c1)
> -				goto next;
> -			if (!c1->destroy(c1)) {
> +			if (c1) {
> +				bret = c1->destroy(c1);
> +			} else {
> +				counter++;
> +				continue;
> +			}
> +			if (!bret) {
>  				fprintf(stderr, "Destroying snapshot %s of %s failed\n", lxcname, my_args.name);
>  				lxc_container_put(c1);
>  				free(buf);
>  				return -1;
>  			}
>  			lxc_container_put(c1);
> -next:
>  			counter++;
>  		}
>  		free(buf);
> @@ -203,16 +207,15 @@ next:
>  	ret = snprintf(path, MAXPATHLEN, "%s/%s/snaps", c->config_path, c->name);
>  	if (ret < 0 || ret >= MAXPATHLEN)
>  		return -1;
> -	if (dir_exists(path)) {
> -		if (!c->destroy_with_snapshots(c)) {
> -			fprintf(stderr, "Destroying %s failed\n", my_args.name);
> -			return -1;
> -		}
> -	} else {
> -		if (!c->destroy(c)) {
> -			fprintf(stderr, "Destroying %s failed\n", my_args.name);
> -			return -1;
> -		}
> +
> +	if (dir_exists(path))
> +		bret = c->destroy_with_snapshots(c);
> +	else
> +		bret = c->destroy(c);
> +
> +	if (!bret) {
> +		fprintf(stderr, "Destroying %s failed\n", my_args.name);
> +		return -1;
>  	}
>  
>  	printf("Destroyed container %s including snapshots \n", my_args.name);
> -- 
> 2.6.0
> 
> _______________________________________________
> lxc-devel mailing list
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel