[libvirt] [PATCH v2 9/9] util: error: Put error code messages into an array

[Peter Krempa]

On Thu, Dec 13, 2018 at 17:02:55 +0100, Erik Skultety wrote:
> On Thu, Dec 13, 2018 at 03:48:56PM +0100, Peter Krempa wrote:
> > Simplify adding of new errors by just adding them to the array of
> > messages rather than having to add conversion code.
> >
> > Additionally most of the messages add the format string part as a suffix
> > so we can avoid some of the duplication by using a macro which adds the
> > suffix to the original string. This way most messages fit into the 80
> > column limit and only 3 exceed 100 colums.
> >
> > Signed-off-by: Peter Krempa <pkrempa at redhat.com>
> > ---
> >
> > Notes:
> >     v2:
> >     - use positional initializers
> >     - add macros for shortening the messages
> >     - make it gettext-friendly, since the last version was not
> >
> >  src/libvirt_private.syms |   1 +
> >  src/util/virerror.c      | 738 +++++++--------------------------------
> >  2 files changed, 126 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;

This hunk will be dropped.

> >  virErrorPreserveLast;
> >  virErrorRestore;
> >  virErrorSetErrnoFromLastError;
> > diff --git a/src/util/virerror.c b/src/util/virerror.c
> > index 03166d85d2..d3f1d7d0e1 100644
> > --- a/src/util/virerror.c
> > +++ b/src/util/virerror.c
> > @@ -904,6 +904,124 @@ void virRaiseErrorObject(const char *filename,
> >  }
> >
> >
> > +typedef struct {
> > +    const char *msg;
> > +    const char *msginfo;
> > +} virErrorMsgTuple;
> > +
> > +#define MSG(msg, sfx) \
> > +   { N_(msg), N_(msg sfx) }
> 
> So ^this is the majority of messages, therefore I think we could introduce one
> more macro MSG_FULL which the ones you introduced could link to and we might
> get rid of the ": %s" suffix which is repeated over and over again.

Soo, will it be okay with the following macro definitions? I've also
added some explanation:

/**
 * These macros expand to the following format of error message:
 * MSG2("error message", " suffix %s")
 *   - no info: "error message"
 *   - info: "error message suffix %s"
 *
 * MSG("error message")
 *  - no info: "error message"
 *  - info: "error message: %s"
 *
 * MSG_EXISTS("sausage")
 *  - no info: "this sausage exists already"
 *  - info: "sausage %s exists already"
 */
#define MSG2(msg, sfx) \
   { N_(msg), N_(msg sfx) }
#define MSG(msg) \
    MSG2(msg, ": %s")
#define MSG_EXISTS(object) \
   { N_("this " object " exists already"), N_(object " %s exists already") }

Followed by the appropriate changes:
   [VIR_ERR_NO_MEMORY] = MSG("out of memory"),
   [VIR_ERR_NO_CONNECT] = MSG2("no connection driver available", " for %s"),

> 
> Since you incorporated Dan's points, there are no further comments from my
> side:
> 
> Reviewed-by: Erik Skultety <eskultet at redhat.com>
> 
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20181214/a8994c13/attachment-0001.sig>