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

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"

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

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