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

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(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; 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")