[libvirt] [PATCH 1/5] internal: Introduce virCheckNonEmptyStringArgGoto and reuse it

[Peter Krempa]

On Thu, Jun 25, 2015 at 12:23:43 -0400, John Ferlan wrote:
> 
> 
> On 06/23/2015 01:15 PM, Peter Krempa wrote:
> > The helper makes sure that strings passed to APIs are non-NULL and
> > non-empty. This allows to drop some inlined checks where it does not
> > make sense.
> > ---
> >  src/internal.h         | 11 +++++++++++
> >  src/libvirt-domain.c   |  4 ++--
> >  src/qemu/qemu_driver.c | 11 -----------
> >  src/util/virerror.h    | 11 +++++++++++
> >  4 files changed, 24 insertions(+), 13 deletions(-)
> > 
> 
> ...
> 
> > diff --git a/src/util/virerror.h b/src/util/virerror.h
> > index ad3a946..c1a445e 100644
> > --- a/src/util/virerror.h
> > +++ b/src/util/virerror.h
> > @@ -95,6 +95,17 @@ void virReportSystemErrorFull(int domcode,
> >                        0, 0,                                          \
> >                        _("%s in %s must not be NULL"),                \
> >                        #argname, __FUNCTION__)
> > +# define virReportInvalidEmptyStringArg(argname)                     \
> > +    virRaiseErrorFull(__FILE__, __FUNCTION__, __LINE__,              \
> > +                      VIR_FROM_THIS,                                 \
> > +                      VIR_ERR_INVALID_ARG,                           \
> > +                      VIR_ERR_ERROR,                                 \
> > +                      __FUNCTION__,                                  \
> > +                      #argname,                                      \
> > +                      NULL,                                          \
> > +                      0, 0,                                          \
> > +                      _("string %s in %s must not be non empty"),    \
> 
> This reads strangely...  The double negative (not and non)
> 
> "string %s in %s must not be empty" or "string %s in %s must be non empty"

I went with the first option. I goofed when I tried to touch up the
message after stealing it from an existing macro :)

> 
> Or more technically
> 
> "string %s in %s cannot start with the NUL character" (yes, correct
> spelling for NUL - that's the '\0')

Yep, but this option does not sound user-friendly to me.

> 
> 
> > +                      #argname, __FUNCTION__)
> >  # define virReportInvalidPositiveArg(argname)                        \
> >      virRaiseErrorFull(__FILE__, __FUNCTION__, __LINE__,              \
> >                        VIR_FROM_THIS,                                 \
> > 
> 
> ACK with some sort of change

Thanks.

Peter
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150626/caeefead/attachment-0001.sig>