Re: [Qemu-devel] [PATCH V8 3/8] util: add error_append()

[Wenchao Xia]

于 2014/1/3 23:32, Peter Crosthwaite 写道:

On Fri, Jan 3, 2014 at 1:08 PM, Wenchao Xia <xiaw...@linux.vnet.ibm.com> wrote:

Signed-off-by: Wenchao Xia <xiaw...@linux.vnet.ibm.com>
---
  include/qapi/error.h |    6 ++++++
  util/error.c         |   21 +++++++++++++++++++++
  2 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/include/qapi/error.h b/include/qapi/error.h
index 7d4c696..bcfb724 100644
--- a/include/qapi/error.h
+++ b/include/qapi/error.h
@@ -30,6 +30,12 @@ typedef struct Error Error;
  void error_set(Error **err, ErrorClass err_class, const char *fmt, ...) 
GCC_FMT_ATTR(3, 4);

  /**
+ * Append message to err if err != NULL && *err != NULL. "\n" will be inserted
+ * after old message.
+ */
+void error_append(Error **err, const char *fmt, ...) GCC_FMT_ATTR(2, 3);
+
+/**
   * Set an indirect pointer to an error given a ErrorClass value and a
   * printf-style human message, followed by a strerror() string if
   * @os_error is not zero.
diff --git a/util/error.c b/util/error.c
index 3ee362a..64bbb2d 100644
--- a/util/error.c
+++ b/util/error.c
@@ -46,6 +46,27 @@ void error_set(Error **errp, ErrorClass err_class, const 
char *fmt, ...)
      errno = saved_errno;
  }

+void error_append(Error **err, const char *fmt, ...)

errp

+{
+    va_list ap;
+    gchar *msg, *msg_old;
+
+    if (!err || !*err) {

Should appending to an unset error really be a nop? You should just
set the error as normal in this case (error_set()?).

This avoids callers having to:

if (error_is_set(&err)) {
    error_append(&err);
} else {
    error_set(&err);
}

so they can just append if they want appending.

Generally speaking, appending to nothing should give you something.

Regards,
Peter

  Make sense, will fix as above, thanks for review.

+        return;
+    }
+
+    va_start(ap, fmt);
+    msg = g_strdup_vprintf(fmt, ap);
+    va_end(ap);
+
+    msg_old = (*err)->msg;
+    (*err)->msg = g_strdup_printf("%s\n%s", msg_old, msg);
+
+    g_free(msg);
+    g_free(msg_old);
+
+}
+
  void error_set_errno(Error **errp, int os_errno, ErrorClass err_class,
                       const char *fmt, ...)
  {
--
1.7.1