Patchwork [2/6] snapshot: add error set function

login
register
mail settings
Submitter Wayne Xia
Date Dec. 17, 2012, 6:25 a.m.
Message ID <1355725509-5429-3-git-send-email-xiawenc@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/206779/
State New
Headers show

Comments

Wayne Xia - Dec. 17, 2012, 6:25 a.m.
Added two function which will try replace the error if it is
already set, so only last error is reported.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 error.c |   23 +++++++++++++++++++++++
 error.h |    9 +++++++++
 2 files changed, 32 insertions(+), 0 deletions(-)
Eric Blake - Dec. 20, 2012, 9:36 p.m.
On 12/16/2012 11:25 PM, Wenchao Xia wrote:
>   Added two function which will try replace the error if it is
> already set, so only last error is reported.
> 

> +#define error_setg_replace(err, fmt, ...) do {                     \
> +    if (*err != NULL) { \
> +        error_free(*err); \
> +     } \
> +    error_set(err, ERROR_CLASS_GENERIC_ERROR, fmt, ## __VA_ARGS__); \
> +} while (/*CONSTCOND*/0)

qemu-queue.h is the only other file in qemu.git that uses /*CONSTCOND*/
notation, and that's because of the file origins; do we really need it here?
Wayne Xia - Dec. 21, 2012, 2:37 a.m.
于 2012-12-21 5:36, Eric Blake 写道:
> On 12/16/2012 11:25 PM, Wenchao Xia wrote:
>>    Added two function which will try replace the error if it is
>> already set, so only last error is reported.
>>
>
>> +#define error_setg_replace(err, fmt, ...) do {                     \
>> +    if (*err != NULL) { \
>> +        error_free(*err); \
>> +     } \
>> +    error_set(err, ERROR_CLASS_GENERIC_ERROR, fmt, ## __VA_ARGS__); \
>> +} while (/*CONSTCOND*/0)
>
> qemu-queue.h is the only other file in qemu.git that uses /*CONSTCOND*/
> notation, and that's because of the file origins; do we really need it here?
>
  OK, /*CONSTCOND*/ will be removed.
Stefan Hajnoczi - Jan. 4, 2013, 2:55 p.m.
On Mon, Dec 17, 2012 at 02:25:05PM +0800, Wenchao Xia wrote:

This patch has nothing to do with snapshots, so "snapshot: add error set
function" is not a useful commit message.  "error: add
error_set_replace()" would be okay.  Please use git log <filename> on
the file you are modifying to find good component names (e.g. "block:",
"error:").

> @@ -29,6 +29,9 @@ typedef struct Error Error;
>   */
>  void error_set(Error **err, ErrorClass err_class, const char *fmt, ...) GCC_FMT_ATTR(3, 4);
>  
> +void error_set_replace(Error **err, ErrorClass err_class, const char *fmt, ...)
> +GCC_FMT_ATTR(3, 4);
> +
>  /**
>   * Set an indirect pointer to an error given a ErrorClass value and a
>   * printf-style human message, followed by a strerror() string if
> @@ -43,6 +46,12 @@ void error_set_errno(Error **err, int os_error, ErrorClass err_class, const char
>      error_set(err, ERROR_CLASS_GENERIC_ERROR, fmt, ## __VA_ARGS__)
>  #define error_setg_errno(err, os_error, fmt, ...) \
>      error_set_errno(err, os_error, ERROR_CLASS_GENERIC_ERROR, fmt, ## __VA_ARGS__)
> +#define error_setg_replace(err, fmt, ...) do {                     \
> +    if (*err != NULL) { \
> +        error_free(*err); \
> +     } \
> +    error_set(err, ERROR_CLASS_GENERIC_ERROR, fmt, ## __VA_ARGS__); \
> +} while (/*CONSTCOND*/0)

Why not use error_set_replace() to get rid of the error_free() check?

Stefan
Wayne Xia - Jan. 5, 2013, 8:27 a.m.
于 2013-1-4 22:55, Stefan Hajnoczi 写道:
> On Mon, Dec 17, 2012 at 02:25:05PM +0800, Wenchao Xia wrote:
>
> This patch has nothing to do with snapshots, so "snapshot: add error set
> function" is not a useful commit message.  "error: add
> error_set_replace()" would be okay.  Please use git log <filename> on
> the file you are modifying to find good component names (e.g. "block:",
> "error:").
>
>> @@ -29,6 +29,9 @@ typedef struct Error Error;
>>    */
>>   void error_set(Error **err, ErrorClass err_class, const char *fmt, ...) GCC_FMT_ATTR(3, 4);
>>
>> +void error_set_replace(Error **err, ErrorClass err_class, const char *fmt, ...)
>> +GCC_FMT_ATTR(3, 4);
>> +
>>   /**
>>    * Set an indirect pointer to an error given a ErrorClass value and a
>>    * printf-style human message, followed by a strerror() string if
>> @@ -43,6 +46,12 @@ void error_set_errno(Error **err, int os_error, ErrorClass err_class, const char
>>       error_set(err, ERROR_CLASS_GENERIC_ERROR, fmt, ## __VA_ARGS__)
>>   #define error_setg_errno(err, os_error, fmt, ...) \
>>       error_set_errno(err, os_error, ERROR_CLASS_GENERIC_ERROR, fmt, ## __VA_ARGS__)
>> +#define error_setg_replace(err, fmt, ...) do {                     \
>> +    if (*err != NULL) { \
>> +        error_free(*err); \
>> +     } \
>> +    error_set(err, ERROR_CLASS_GENERIC_ERROR, fmt, ## __VA_ARGS__); \
>> +} while (/*CONSTCOND*/0)
>
> Why not use error_set_replace() to get rid of the error_free() check?
>
> Stefan
>

   It will be changed to error_set_check() which only set error when
*err is not set, to keep first error not over written.

Patch

diff --git a/error.c b/error.c
index 128d88c..5a82f8e 100644
--- a/error.c
+++ b/error.c
@@ -43,6 +43,29 @@  void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...)
     *errp = err;
 }
 
+void error_set_replace(Error **errp, ErrorClass err_class, const char *fmt, ...)
+{
+    Error *err;
+    va_list ap;
+
+    if (errp == NULL) {
+        return;
+    }
+    if (*errp != NULL) {
+        error_free(*errp);
+        *errp = NULL;
+    }
+
+    err = g_malloc0(sizeof(*err));
+
+    va_start(ap, fmt);
+    err->msg = g_strdup_vprintf(fmt, ap);
+    va_end(ap);
+    err->err_class = err_class;
+
+    *errp = err;
+}
+
 void error_set_errno(Error **errp, int os_errno, ErrorClass err_class,
                      const char *fmt, ...)
 {
diff --git a/error.h b/error.h
index 4d52e73..8039408 100644
--- a/error.h
+++ b/error.h
@@ -29,6 +29,9 @@  typedef struct Error Error;
  */
 void error_set(Error **err, ErrorClass err_class, const char *fmt, ...) GCC_FMT_ATTR(3, 4);
 
+void error_set_replace(Error **err, ErrorClass err_class, const char *fmt, ...)
+GCC_FMT_ATTR(3, 4);
+
 /**
  * Set an indirect pointer to an error given a ErrorClass value and a
  * printf-style human message, followed by a strerror() string if
@@ -43,6 +46,12 @@  void error_set_errno(Error **err, int os_error, ErrorClass err_class, const char
     error_set(err, ERROR_CLASS_GENERIC_ERROR, fmt, ## __VA_ARGS__)
 #define error_setg_errno(err, os_error, fmt, ...) \
     error_set_errno(err, os_error, ERROR_CLASS_GENERIC_ERROR, fmt, ## __VA_ARGS__)
+#define error_setg_replace(err, fmt, ...) do {                     \
+    if (*err != NULL) { \
+        error_free(*err); \
+     } \
+    error_set(err, ERROR_CLASS_GENERIC_ERROR, fmt, ## __VA_ARGS__); \
+} while (/*CONSTCOND*/0)
 
 /**
  * Returns true if an indirect pointer to an error is pointing to a valid