[libvirt] [PATCH 08/10] util: error: Use a more declarative approach in virErrorMsg

[Daniel P. Berrangé]

On Thu, Dec 06, 2018 at 11:42:44AM +0000, Daniel P. Berrangé wrote:
> On Wed, Dec 05, 2018 at 05:47:49PM +0100, Peter Krempa wrote:
> > Use a macro to declare how the strings for individual error codes. This
> > unifies the used condition and will allow simplifying the code further.
> > 
> > Signed-off-by: Peter Krempa <pkrempa at redhat.com>
> > ---
> >  src/libvirt_private.syms |   1 +
> >  src/util/virerror.c      | 792 +++++++++------------------------------
> >  src/util/virerrorpriv.h  |   8 +
> >  3 files changed, 188 insertions(+), 613 deletions(-)
> > 
> > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> > index 6184030d59..775b33e151 100644
> > --- a/src/libvirt_private.syms
> > +++ b/src/libvirt_private.syms
> > @@ -1753,6 +1753,7 @@ virDispatchError;
> >  virErrorCopyNew;
> >  virErrorInitialize;
> >  virErrorMsg;
> > +virErrorMsgStrings;
> >  virErrorPreserveLast;
> >  virErrorRestore;
> >  virErrorSetErrnoFromLastError;
> > diff --git a/src/util/virerror.c b/src/util/virerror.c
> > index 7444d671bb..d3cd06331f 100644
> > --- a/src/util/virerror.c
> > +++ b/src/util/virerror.c
> > @@ -903,6 +903,178 @@ void virRaiseErrorObject(const char *filename,
> >  }
> > 
> > 
> > +const virErrorMsgTuple virErrorMsgStrings[VIR_ERR_NUMBER_LAST] = {
> > +    { VIR_ERR_OK, NULL, NULL },
> > +    { VIR_ERR_INTERNAL_ERROR, "internal error", "internal error: %s" },
> > +    { VIR_ERR_NO_MEMORY, "out of memory", "out of memory: %s" },
> > +    { VIR_ERR_NO_SUPPORT,
> > +        "this function is not supported by the connection driver",
> > +        "this function is not supported by the connection driver: %s" },
> 
> The vast majority of messages have identical text apart from a small
> suffix. How about using a macro to kill the duplication in the source
> eg
> 
>  #define MSG(msg, suffix) \
>       msg, msg # suffix
> 
>   { VIR_ERR_NO_SUPPORT,
>     MSG("this function is not supported by the connection driver", ": %s") },
> 
> Then, only a handful will need separate entries

I've unfortunately just realized this was an absolutely terrible
suggestion of mine. We cannot use CPP string concatenation here
as it breaks xgettext's ability to discover translatable strings.
IOW we just lost all our error messages from the translations :-(

I've posted a patch that puts it back to roughly what you have in
this v1 patch.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|