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@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@redhat.com>
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list