[libvirt] [PATCH 1/4] internal: introduce macro virCheckControllerGoto

[Daniel P. Berrange]

On Wed, Nov 23, 2016 at 10:49:00AM -0500, John Ferlan wrote:
> 
> 
> On 11/03/2016 11:09 PM, Chen Hanxiao wrote:
> > From: Chen Hanxiao <chenhanxiao at gmail.com>
> > 
> > Introduce macro virCheckControllerGoto.
> > Jumps to a label if unsupported controller type were
> > passed to it.
> > 
> > Signed-off-by: Chen Hanxiao <chenhanxiao at gmail.com>
> > ---
> >  src/internal.h | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> > 
> > diff --git a/src/internal.h b/src/internal.h
> > index d8cc5ad..a34f43e 100644
> > --- a/src/internal.h
> > +++ b/src/internal.h
> > @@ -362,6 +362,25 @@
> >          }                                                               \
> >      } while (0)
> >  
> > +/**
> > + * virCheckControllerGoto:
> > + * @Cgp: virCgroupPtr pointer
> > + * @CgpCtr: cgroup controller type enum
> > + * @label: label to jump to on error
> > + *
> > + * Returns nothing. Jumps to a label if unsupported controller type were
> > + * passed to it.
> > + */
> > +# define virCheckControllerGoto(Cgp, CgpCtr, label)                    \
> > +    do {                                                               \
> > +        if (!virCgroupHasController(Cgp, CgpCtr)) {                    \
> > +            virReportError(VIR_ERR_OPERATION_INVALID,                  \
> > +                            _("cgroup %s controller is not mounted"),  \
>                               ^
> Extra space
> 
> s/%s/'%s'/
> 
> ACK (I'll adjust before pushing)

I'm personally *not* in favour of doing this. I don't think that macros
should be used to hide control flow logic, especially when they're hiding
'goto'.

Now, we do have some of this style macros for checking flags. I think those
are more acceptable because flag checking is always done right at the
start of the function.

These cgroup macros by contrast will be placed at arbitrary locations
in the middle of functions, hiding the fact that we're jumping right
out.

So personally I'm NACK on this series, unless there contrasting opinions
people want to put forward.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|