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 <[email protected]> > > --- > > > > 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 <[email protected]>
>
> --
> libvir-list mailing list
> [email protected]
> https://www.redhat.com/mailman/listinfo/libvir-list
signature.asc
Description: PGP signature
-- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
