On Tue, Mar 24, 2015 at 04:40:17PM +0100, Peter Krempa wrote:
> On Fri, Mar 20, 2015 at 15:38:59 +0100, Pavel Hrdina wrote:
> > Inspired by commit 7e437ee7 that introduced similar macros for virsh
> > commands so we don't have to repeat the same code all over.
> >
> > Signed-off-by: Pavel Hrdina <phrdina@xxxxxxxxxx>
> > ---
> > src/internal.h | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 80 insertions(+)
> >
> > diff --git a/src/internal.h b/src/internal.h
> > index 4d473af..5885f03 100644
> > --- a/src/internal.h
> > +++ b/src/internal.h
> > @@ -327,6 +327,86 @@
> > } \
> > } while (0)
> >
> > +/* Macros to help dealing with mutually exclusive flags. */
> > +
> > +/**
> > + * VIR_EXCLUSIVE_FLAGS_EXPR_RET:
> > + *
> > + * @NAME1: String containing the name of the flag.
> > + * @EXPR1: Expression to validate the flag.
> > + * @NAME2: String containing the name of the flag.
> > + * @EXPR2: Expression to validate the flag.
> > + *
> > + * Reject mutually exclusive API flags. Use the provided expression
> > + * to check the flags.
> > + *
> > + * This helper does an early return and therefore it has to be called
> > + * before anything that would require cleanup.
> > + */
> > +# define VIR_EXCLUSIVE_FLAGS_EXPR_RET(NAME1, EXPR1, NAME2, EXPR2) \
> > + if ((EXPR1) && (EXPR2)) { \
> > + virReportInvalidArg(ctl, \
> > + _("Flags '%s' and '%s' are mutually exclusive"),\
> > + NAME1, NAME2); \
> > + return -1; \
> > + }
> > +
>
> While in virsh the above variant makes sense, because in some cases the
> variables have different names than the corresponding flags, in case of
> this code having it doesn't make sense.
You're right. I've only used the other macros.
>
>
> > +/**
> > + * VIR_EXCLUSIVE_FLAGS_VAR_RET:
> > + *
> > + * @FLAG1: First flag to be checked
> > + * @FLAG2: Second flag to be checked
>
> A third argument @RET should be added here so that this can be used in
> places that also return NULL or any other possible variable.
>
> In virsh the assumption is that the flags are checked at first and thus
> returning false makes sense, as every virsh command does that.
I'll add the @RET argument.
>
> > + *
> > + * Reject mutually exclusive API flags. The checked flags are compared
> > + * with flags variable.
> > + *
> > + * This helper does an early return and therefore it has to be called
> > + * before anything that would require cleanup.
> > + */
> > +# define VIR_EXCLUSIVE_FLAGS_RET(FLAG1, FLAG2) \
> > + VIR_EXCLUSIVE_FLAGS_EXPR_RET(#FLAG1, flags & FLAG1, #FLAG2, flags & FLAG2)
> > +
> > +/**
> > + * VIR_EXCLUSIVE_FLAGS_EXPR_GOTO:
> > + *
> > + * @NAME1: String containing the name of the flag.
> > + * @EXPR1: Expression to validate the flag.
> > + * @NAME2: String containing the name of the flag.
> > + * @EXPR2: Expression to validate the flag.
> > + * @LABEL: label to jump to.
> > + *
> > + * Reject mutually exclusive API flags. Use the provided expression
> > + * to check the flag.
> > + *
> > + * Returns nothing. Jumps to a label if unsupported flags were
> > + * passed to it.
> > + */
> > +# define VIR_EXCLUSIVE_FLAGS_EXPR_GOTO(NAME1, EXPR1, NAME2, EXPR2, LABEL) \
> > + if ((EXPR1) && (EXPR2)) { \
> > + virReportInvalidArg(ctl, \
> > + _("Flags '%s' and '%s' are mutually exclusive"),\
> > + NAME1, NAME2); \
> > + goto LABEL; \
> > + }
> > +
>
> Again, this function does not make sense in the library code.
>
> > +/**
> > + * VIR_EXCLUSIVE_FLAGS_VAR_GOTO:
> > + *
> > + * @FLAG1: First flag to be checked.
> > + * @FLAG2: Second flag to be checked.
> > + * @LABEL: label to jump to.
> > + *
> > + * Reject mutually exclusive API flags. The checked flags are compared
> > + * with flags variable.
> > + *
> > + * Returns nothing. Jumps to a label if unsupported flags were
> > + * passed to it.
> > + */
> > +# define VIR_EXCLUSIVE_FLAGS_GOTO(FLAG1, FLAG2, LABEL) \
> > + VIR_EXCLUSIVE_FLAGS_EXPR_GOTO(#FLAG1, flags & FLAG1, \
> > + #FLAG2, flags & FLAG2, LABEL)
> > +
> > +
> > # define virCheckNonNullArgReturn(argname, retval) \
> > do { \
> > if (argname == NULL) { \
>
> While this is okay.
>
> Peter
Thanks for review, I'll send v2.
Pavel
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list