Re: [PATCH 2/5] virerror: Fix incorrect use of RaiseErrorFull

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

 



On Sat, May 03, 2014 at 03:59:39PM -0400, Cole Robinson wrote:
> RaiseErrorFull does not prepend the static error code string (like
> INVALID_ARG yields "invalid arg: %(msg)s"). We should be using
> ReportErrorHelper.
> 
> The generated error objects are slightly different, by not storing the
> invalid argument name in err->str2. However those fields aren't used
> anywhere else and aren't documented to contain anything useful, so
> I don't think it matters.
> ---
>  src/util/virerror.h | 116 +++++++++++++++++-----------------------------------
>  1 file changed, 38 insertions(+), 78 deletions(-)
> 
> diff --git a/src/util/virerror.h b/src/util/virerror.h
> index fe0e15e..872c270 100644
> --- a/src/util/virerror.h
> +++ b/src/util/virerror.h
> @@ -69,92 +69,52 @@ void virReportSystemErrorFull(int domcode,
>                               (fmt), __VA_ARGS__)
>  
>  # define virReportInvalidNullArg(argname)                            \
> -    virRaiseErrorFull(__FILE__, __FUNCTION__, __LINE__,              \
> -                      VIR_FROM_THIS,                                 \
> -                      VIR_ERR_INVALID_ARG,                           \
> -                      VIR_ERR_ERROR,                                 \
> -                      __FUNCTION__,                                  \
> -                      #argname,                                      \
> -                      NULL,                                          \
> -                      0, 0,                                          \
> -                      _("%s in %s must be NULL"),                    \
> -                      #argname, __FUNCTION__)
> +    virReportErrorHelper(VIR_FROM_THIS,                              \
> +                         VIR_ERR_INVALID_ARG,                        \
> +                         __FILE__, __FUNCTION__, __LINE__,           \
> +                         _("%s in %s must be NULL"),                 \
> +                         #argname, __FUNCTION__)
>  # define virReportInvalidNonNullArg(argname)                         \
> -    virRaiseErrorFull(__FILE__, __FUNCTION__, __LINE__,              \
> -                      VIR_FROM_THIS,                                 \
> -                      VIR_ERR_INVALID_ARG,                           \
> -                      VIR_ERR_ERROR,                                 \
> -                      __FUNCTION__,                                  \
> -                      #argname,                                      \
> -                      NULL,                                          \
> -                      0, 0,                                          \
> -                      _("%s in %s must not be NULL"),                \
> -                      #argname, __FUNCTION__)
> +    virReportErrorHelper(VIR_FROM_THIS,                              \
> +                         VIR_ERR_INVALID_ARG,                        \
> +                         __FILE__, __FUNCTION__, __LINE__,           \
> +                         _("%s in %s must not be NULL"),             \
> +                         #argname, __FUNCTION__)
>  # define virReportInvalidPositiveArg(argname)                        \
> -    virRaiseErrorFull(__FILE__, __FUNCTION__, __LINE__,              \
> -                      VIR_FROM_THIS,                                 \
> -                      VIR_ERR_INVALID_ARG,                           \
> -                      VIR_ERR_ERROR,                                 \
> -                      __FUNCTION__,                                  \
> -                      #argname,                                      \
> -                      NULL,                                          \
> -                      0, 0,                                          \
> -                      _("%s in %s must be greater than zero"),       \
> -                      #argname, __FUNCTION__)
> +    virReportErrorHelper(VIR_FROM_THIS,                              \
> +                         VIR_ERR_INVALID_ARG,                        \
> +                         __FILE__, __FUNCTION__, __LINE__,           \
> +                         _("%s in %s must be greater than zero"),    \
> +                         #argname, __FUNCTION__)
>  # define virReportInvalidNonZeroArg(argname)                         \
> -    virRaiseErrorFull(__FILE__, __FUNCTION__, __LINE__,              \
> -                      VIR_FROM_THIS,                                 \
> -                      VIR_ERR_INVALID_ARG,                           \
> -                      VIR_ERR_ERROR,                                 \
> -                      __FUNCTION__,                                  \
> -                      #argname,                                      \
> -                      NULL,                                          \
> -                      0, 0,                                          \
> -                      _("%s in %s must not be zero"),                \
> -                      #argname, __FUNCTION__)
> +    virReportErrorHelper(VIR_FROM_THIS,                              \
> +                         VIR_ERR_INVALID_ARG,                        \
> +                         __FILE__, __FUNCTION__, __LINE__,           \
> +                         _("%s in %s must not be zero"),             \
> +                         #argname, __FUNCTION__)
>  # define virReportInvalidZeroArg(argname)                            \
> -    virRaiseErrorFull(__FILE__, __FUNCTION__, __LINE__,              \
> -                      VIR_FROM_THIS,                                 \
> -                      VIR_ERR_INVALID_ARG,                           \
> -                      VIR_ERR_ERROR,                                 \
> -                      __FUNCTION__,                                  \
> -                      #argname,                                      \
> -                      NULL,                                          \
> -                      0, 0,                                          \
> -                      _("%s in %s must be zero"),                    \
> -                      #argname, __FUNCTION__)
> +    virReportErrorHelper(VIR_FROM_THIS,                              \
> +                         VIR_ERR_INVALID_ARG,                        \
> +                         __FILE__, __FUNCTION__, __LINE__,           \
> +                         _("%s in %s must be zero"),                 \
> +                         #argname, __FUNCTION__)
>  # define virReportInvalidNonNegativeArg(argname)                     \
> -    virRaiseErrorFull(__FILE__, __FUNCTION__, __LINE__,              \
> -                      VIR_FROM_THIS,                                 \
> -                      VIR_ERR_INVALID_ARG,                           \
> -                      VIR_ERR_ERROR,                                 \
> -                      __FUNCTION__,                                  \
> -                      #argname,                                      \
> -                      NULL,                                          \
> -                      0, 0,                                          \
> -                      _("%s in %s must be zero or greater"),         \
> -                      #argname, __FUNCTION__)
> +    virReportErrorHelper(VIR_FROM_THIS,                              \
> +                         VIR_ERR_INVALID_ARG,                        \
> +                         __FILE__, __FUNCTION__, __LINE__,           \
> +                         _("%s in %s must be zero or greater"),      \
> +                         #argname, __FUNCTION__)
>  # define virReportInvalidArg(argname, fmt, ...)                      \
> -    virRaiseErrorFull(__FILE__, __FUNCTION__, __LINE__,              \
> -                      VIR_FROM_THIS,                                 \
> -                      VIR_ERR_INVALID_ARG,                           \
> -                      VIR_ERR_ERROR,                                 \
> -                      __FUNCTION__,                                  \
> -                      #argname,                                      \
> -                      NULL,                                          \
> -                      0, 0,                                          \
> -                      (fmt), __VA_ARGS__)
> +    virReportErrorHelper(VIR_FROM_THIS,                              \
> +                         VIR_ERR_INVALID_ARG,                        \
> +                         __FILE__, __FUNCTION__, __LINE__,           \
> +                         (fmt), __VA_ARGS__)
>  
>  # define virReportDBusServiceError(message, name)                    \
> -    virRaiseErrorFull(__FILE__, __FUNCTION__, __LINE__,              \
> -                      VIR_FROM_THIS,                                 \
> -                      VIR_ERR_DBUS_SERVICE,                          \
> -                      VIR_ERR_ERROR,                                 \
> -                      __FUNCTION__,                                  \
> -                      name,                                          \
> -                      NULL,                                          \
> -                      0, 0,                                          \
> -                      "%s", message);
> +    virReportErrorHelper(VIR_FROM_THIS,                              \
> +                         VIR_ERR_DBUS_SERVICE,                       \
> +                         __FILE__, __FUNCTION__, __LINE__,           \
> +                         "%s", message)
>  
>  # define virReportUnsupportedError()                                    \
>      virReportErrorHelper(VIR_FROM_THIS, VIR_ERR_NO_SUPPORT,             \
> -- 

I do loke this better but might not grasp all implications.
 -- Guido

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]