subject:"\[libvirt\] \[PATCH v2 9\/9\] util\: error\: Put error code messages into an array"

Re: [libvirt] [PATCH v2 9/9] util: error: Put error code messages into an array

2018-12-14 Thread Erik Skultety

On Fri, Dec 14, 2018 at 10:35:50AM +0100, Peter Krempa wrote:
> 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 
> > > ---
> > >
> > > 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")

if you make ^this N_(msg) then sure.

Erik

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

Re: [libvirt] [PATCH v2 9/9] util: error: Put error code messages into an array

2018-12-14 Thread Peter Krempa

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 
> > ---
> >
> > 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 
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2 9/9] util: error: Put error code messages into an array

2018-12-13 Thread Erik Skultety

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

Since you incorporated Dan's points, there are no further comments from my
side:

Reviewed-by: Erik Skultety 

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

[libvirt] [PATCH v2 9/9] util: error: Put error code messages into an array

2018-12-13 Thread Peter Krempa

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 
---

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;
 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) }
+#define MSG_EXISTS(object) \
+   { N_("this " object " exists already"), N_(object " %s exists already") }
+
+const virErrorMsgTuple virErrorMsgStrings[VIR_ERR_NUMBER_LAST] = {
+[VIR_ERR_OK] = { NULL, NULL },
+[VIR_ERR_INTERNAL_ERROR] = MSG("internal error", ": %s"),
+[VIR_ERR_NO_MEMORY] = MSG("out of memory", ": %s"),
+[VIR_ERR_NO_SUPPORT] = MSG("this function is not supported by the 
connection driver", ": %s"),
+[VIR_ERR_UNKNOWN_HOST] = MSG("unknown host", " %s"),
+[VIR_ERR_NO_CONNECT] = MSG("no connection driver available", " for %s"),
+[VIR_ERR_INVALID_CONN] = MSG("invalid connection pointer in", " %s"),
+[VIR_ERR_INVALID_DOMAIN] = MSG("invalid domain pointer in", " %s"),
+[VIR_ERR_INVALID_ARG] = MSG("invalid argument", ": %s"),
+[VIR_ERR_OPERATION_FAILED] = MSG("operation failed", ": %s"),
+[VIR_ERR_GET_FAILED] = MSG("GET operation failed", ": %s"),
+[VIR_ERR_POST_FAILED] = MSG("POST operation failed", ": %s"),
+[VIR_ERR_HTTP_ERROR] = MSG("got unknown HTTP error code", " %s"),
+[VIR_ERR_SEXPR_SERIAL] = MSG("failed to serialize S-Expr", ": %s"),
+[VIR_ERR_NO_XEN] = MSG("could not use Xen hypervisor entry", " %s"),
+[VIR_ERR_XEN_CALL] = MSG("failed Xen syscall", " %s"),
+[VIR_ERR_OS_TYPE] = MSG("unknown OS type", " %s"),
+[VIR_ERR_NO_KERNEL] = MSG("missing kernel information", ": %s"),
+[VIR_ERR_NO_ROOT] = MSG("missing root device information", " in %s"),
+[VIR_ERR_NO_SOURCE] = MSG("missing source information for device", " %s"),
+[VIR_ERR_NO_TARGET] = MSG("missing target information for device", " %s"),
+[VIR_ERR_NO_NAME] = MSG("missing name information", " in %s"),
+[VIR_ERR_NO_OS] = MSG("missing operating system information", " for %s"),
+[VIR_ERR_NO_DEVICE] = MSG("missing devices information", " for %s"),
+[VIR_ERR_NO_XENSTORE] = MSG("could not connect to Xen Store", " %s"),
+[VIR_ERR_DRIVER_FULL] = MSG("too many drivers registered", " in %s"),
+[VIR_ERR_CALL_FAILED] = MSG("library call failed", ": %s"),
+[VIR_ERR_XML_ERROR] = { "XML description is invalid or not well formed", 
"XML error: %s" },
+[VIR_ERR_DOM_EXIST] = MSG_EXISTS("domain"),
+[VIR_ERR_OPERATION_DENIED] = { "operation forbidden for read only access", 
"operation forbidden: %s" },
+[VIR_ERR_OPEN_FAILED] = MSG("failed to open configuration file", " %s"),
+[VIR_ERR_READ_FAILED] = MSG("failed to read configuration file", " %s"),
+[VIR_ERR_PARSE_FAILED] = MSG("failed to parse configuration file", " %s"),
+[VIR_ERR_CONF_SYNTAX] = MSG("configuration file syntax error", ": %s"),
+[VIR_ERR_WRITE_FAILED] = MSG("failed to write configuration file", ": %s"),
+[VIR_ERR_XML_DETAIL] = { "parser error", "%s" },
+[VIR_ERR_INVALID_NETWORK] = MSG("invalid network pointer in", " %s"),
+[VIR_ERR_NETWORK_EXIST] = MSG_EXISTS("network"),
+[VIR_ERR_SYSTEM_ERROR] = { "system call error", "%s" },
+[VIR_ERR_RPC] = { "RPC error", "%s" },
+[VIR_ERR_GNUTLS_ERROR] = { "GNUTLS call error", "%s" },
+[VIR_WAR_NO_NETWORK] = MSG("Failed to find the network", ": %s"),
+[VIR_ERR_NO_DOMAIN] = MSG("Domain not found", ": %s"),
+[VIR_ERR_NO_NETWORK] = MSG("Network not found", ": %s"),
+[VIR_ERR_INVALID_MAC] = MSG("invalid MAC address", ": %s"),
+[VIR_ERR_AUTH_FAILED] = MSG("authentication failed", ": %s"),
+[VIR_ERR_INVALID_STORAGE_POOL] = MSG("invalid storage pool pointer in", " 
%s"),
+[VIR_ERR_INVALID_STORAGE_VOL] = MSG("invalid storage volume pointer in", " 
%s"),
+

Re: [libvirt] [PATCH v2 9/9] util: error: Put error code messages into an array

Re: [libvirt] [PATCH v2 9/9] util: error: Put error code messages into an array

Re: [libvirt] [PATCH v2 9/9] util: error: Put error code messages into an array

[libvirt] [PATCH v2 9/9] util: error: Put error code messages into an array

4 matches

Site Navigation

Mail list logo

Footer information