[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v2 1/2] Introduce virCheckFlags for consistent flags checking



> > The idea is that every API implementation in driver which has flags
> > parameter should first call virCheckFlags() macro to check the function
> > was called with supported flags:
> > 
> >     virCheckFlags(VIR_SUPPORTED_FLAG_1 |
> >                   VIR_SUPPORTED_FLAG_2 |
> >                   VIR_ANOTHER_SUPPORTED_FLAG, -1);
> > 
> > The error massage which is printed when unsupported flags are passed
> > looks like:
> > 
> >     invalid argument in virFooBar: unsupported flags (0x2)
> > 
> > Where the unsupported flags part only prints those flags which were
> > passed but are not supported rather than all flags passed.
> > ---
> >  src/internal.h |   23 +++++++++++++++++++++++
> 
> I like this location better than the v1 attempt.  ACK (but note my
> comments on Matthias' ESX patch, depending on which gets pushed first).

Thanks, I pushed the patch.

> > +/**
> > + * virCheckFlags:
> > + * @supported: an OR'ed set of supported flags
> 
> Is it worth documenting that this must be 'int' or 'unsigned int', and
> that the macro doesn't work on uint64_t?  Or maybe it's worth trying to
> figure out a way to refactor the macro to support both sizes?  But that
> can be a followup, if we find a case where we ever need a larger size
> for flags, so it shouldn't hold up committing this.

I left it as is for now as we only use (unsigned) int flags now. Once we add
an API with 64b flags we can enhance this macro.

Jirka


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]