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

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(a)redhat.com&gt; > --- > > 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;

> 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.

Since you incorporated Dan's points, there are no further comments from my side: Reviewed-by: Erik Skultety <eskultet(a)redhat.com&gt; -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list