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

[Daniel P. Berrangé]

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.

> +    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...
 };


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|