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

[Peter Krempa]

On Thu, Dec 06, 2018 at 11:48:15 +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 at 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" },
> 
> [snip]
> 
> > +    if (info)
> > +        return virErrorMsgStrings[error].msginfo;
> > +    else
> > +        return virErrorMsgStrings[error].msg;
> 
> The code only reads the .msginfo / .msg fields, so...
> 
> >  }
> > 
> > +
> >  /**
> >   * virReportErrorHelper:
> >   *
> > diff --git a/src/util/virerrorpriv.h b/src/util/virerrorpriv.h
> > index bc214393e6..1be40d8a51 100644
> > --- a/src/util/virerrorpriv.h
> > +++ b/src/util/virerrorpriv.h
> > @@ -21,6 +21,14 @@
> >  #ifndef __VIR_ERROR_PRIV_H__
> >  # define __VIR_ERROR_PRIV_H__
> > 
> > +typedef struct {
> > +    virErrorNumber error;
> 
> ..what's the point of storing this which AFAICT just duplicates
> the array index number.

Originally they were in a somewhat random order, so I used a lookup in
the array as an intermediate step in the refactor.

Later I've decided to merge the patches, so they are unused now.
Currently it serves only the purpose to identify the message ...

> 
> > +    const char *msg;
> > +    const char *msginfo;
> > +} virErrorMsgTuple;
> > +
> > +extern const virErrorMsgTuple virErrorMsgStrings[VIR_ERR_NUMBER_LAST];
> 
> Could we change it to just use named array indexes during init eg
> 
>  const virErrorMsgTuple virErrorMsgStrings[VIR_ERR_NUMBER_LAST] = {
>    [VIR_ERR_OK] = {  NULL, NULL },
>    [VIR_ERR_INTERNAL_ERROR = { "internal error", "internal error: %s" },
>    ...etc...

... but that does work here as well.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20181206/6873a9ba/attachment-0001.sig>