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

[Erik Skultety]

On Fri, Dec 14, 2018 at 10:35:50AM +0100, Peter Krempa wrote:
> 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")

if you make ^this N_(msg) then sure.

Erik