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

[Serge Hallyn]

The same anti-goto was originally pounded into me, then I did some kernel
coding and saw how nice it can be compared to alternatives :)

fwiw, the plus side of the goto in this case is to commonize the work to
be done before the next loop (in this case just a 'counter++').  The minus
side is that "what do i do if lxc_container_new failed" isn't all in one
place.  Hence I say in this case it's a draw for me.

This may seem like extraneous chatting, but occasionally coding style
discussion is important - there's no coding style which when used
consistently is nearly as ugly as code which mixes coding styles :)

Quoting Christian Brauner (christianvanbrauner at gmail.com):
> Sorry, my teachers pounded a violent hatred for gotos for non-cleanup purposes
> into me. Thanks. :)
> 
> On Wed, Sep 30, 2015 at 04:13:12PM +0000, Serge Hallyn wrote:
> > 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>
> > 
> > Note I still don't really feel the first hunk is an improvement
> > in readability, but at this point it's subjective, so I won't
> > object :)
> > 
> > Acked-by: Serge E. Hallyn <serge.hallyn at ubuntu.com>
> > 
> > thanks,
> > -serge
> > 
> > > ---
> > >  src/lxc/lxc_destroy.c | 27 ++++++++++++++-------------
> > >  1 file changed, 14 insertions(+), 13 deletions(-)
> > > 
> > > diff --git a/src/lxc/lxc_destroy.c b/src/lxc/lxc_destroy.c
> > > index f1830fd..ab1029f 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,8 +185,10 @@ 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) {
> > > +				counter++;
> > > +				continue;
> > > +			}
> > >  			if (!c1->destroy(c1)) {
> > >  				fprintf(stderr, "Destroying snapshot %s of %s failed\n", lxcname, my_args.name);
> > >  				lxc_container_put(c1);
> > > @@ -193,7 +196,6 @@ static int do_destroy_with_snapshots(struct lxc_container *c)
> > >  				return -1;
> > >  			}
> > >  			lxc_container_put(c1);
> > > -next:
> > >  			counter++;
> > >  		}
> > >  		free(buf);
> > > @@ -203,16 +205,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
> > 
> > _______________________________________________
> > lxc-devel mailing list
> > lxc-devel at lists.linuxcontainers.org
> > http://lists.linuxcontainers.org/listinfo/lxc-devel
> _______________________________________________
> lxc-devel mailing list
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel