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@xxxxxxxxxx>
> > ---
> >
> > 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@xxxxxxxxxx>
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
