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
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list