[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 08/10] util: error: Use a more declarative approach in virErrorMsg



On Thu, Dec 06, 2018 at 11:42:44 +0000, Daniel Berrange wrote:
> On Wed, Dec 05, 2018 at 05:47:49PM +0100, Peter Krempa wrote:
> > Use a macro to declare how the strings for individual error codes. This
> > unifies the used condition and will allow simplifying the code further.
> > 
> > Signed-off-by: Peter Krempa <pkrempa redhat com>
> > ---
> >  src/libvirt_private.syms |   1 +
> >  src/util/virerror.c      | 792 +++++++++------------------------------
> >  src/util/virerrorpriv.h  |   8 +
> >  3 files changed, 188 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 7444d671bb..d3cd06331f 100644
> > --- a/src/util/virerror.c
> > +++ b/src/util/virerror.c
> > @@ -903,6 +903,178 @@ void virRaiseErrorObject(const char *filename,
> >  }
> > 
> > 
> > +const virErrorMsgTuple virErrorMsgStrings[VIR_ERR_NUMBER_LAST] = {
> > +    { VIR_ERR_OK, NULL, NULL },
> > +    { VIR_ERR_INTERNAL_ERROR, "internal error", "internal error: %s" },
> > +    { VIR_ERR_NO_MEMORY, "out of memory", "out of memory: %s" },
> > +    { VIR_ERR_NO_SUPPORT,
> > +        "this function is not supported by the connection driver",
> > +        "this function is not supported by the connection driver: %s" },
> 
> The vast majority of messages have identical text apart from a small
> suffix. How about using a macro to kill the duplication in the source
> eg
> 
>  #define MSG(msg, suffix) \
>       msg, msg # suffix
> 
>   { VIR_ERR_NO_SUPPORT,
>     MSG("this function is not supported by the connection driver", ": %s") },
> 
> Then, only a handful will need separate entries

I thought about this in a less, generic way (by adding a macro that
handles the ": %s" case specifically). I'll try that in the next
submission then. 

Attachment: signature.asc
Description: PGP signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]