diff mbox

[RFC,v1,03/25] error: Factor out common error setter logic

Message ID 7698893fc571c3b988dfff8470ea3edb75b0e593.1441667360.git.crosthwaite.peter@gmail.com
State New
Headers show

Commit Message

Peter Crosthwaite Sept. 11, 2015, 5:33 a.m. UTC 
Currently set_error, set_error_errno, set_error_win32 do pretty much
the same thing with only a little bit of string manipulation in the
error code cases. Coreify that string manipulation as a callback and
factor out the bulk of the error_set fns to a common helper that calls
the callback at the right time.

Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---

 util/error.c | 81 +++++++++++++++++++++++++-----------------------------------
 1 file changed, 33 insertions(+), 48 deletions(-)

Comments

Eric Blake Sept. 11, 2015, 3:30 p.m. UTC | #1 
On 09/10/2015 11:33 PM, Peter Crosthwaite wrote:
> Currently set_error, set_error_errno, set_error_win32 do pretty much
> the same thing with only a little bit of string manipulation in the
> error code cases. Coreify that string manipulation as a callback and
> factor out the bulk of the error_set fns to a common helper that calls
> the callback at the right time.
> 

Commit 5523750 pretty much already does this. This patch can be dropped
when rebasing the series to latest.

> Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
> ---
> 
>  util/error.c | 81 +++++++++++++++++++++++++-----------------------------------
>  1 file changed, 33 insertions(+), 48 deletions(-)
>
diff mbox

Patch

diff --git a/util/error.c b/util/error.c
index 14f4351..b93e5c8 100644
--- a/util/error.c
+++ b/util/error.c
@@ -22,7 +22,9 @@  struct Error
 
 Error *error_abort;
 
-void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...)
+static void do_error_set(Error **errp, ErrorClass err_class,
+                         void (*mod)(Error *, void *), void *mod_opaque,
+                         const char *fmt, ...)
 {
     Error *err;
     va_list ap;
@@ -38,6 +40,9 @@  void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...)
     va_start(ap, fmt);
     err->msg = g_strdup_vprintf(fmt, ap);
     va_end(ap);
+    if (mod) {
+        mod(err, mod_opaque);
+    }
     err->err_class = err_class;
 
     if (errp == &error_abort) {
@@ -50,40 +55,33 @@  void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...)
     errno = saved_errno;
 }
 
-void error_set_errno(Error **errp, int os_errno, ErrorClass err_class,
-                     const char *fmt, ...)
+void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...)
 {
-    Error *err;
-    char *msg1;
     va_list ap;
-    int saved_errno = errno;
 
-    if (errp == NULL) {
-        return;
-    }
-    assert(*errp == NULL);
+    va_start(ap, fmt);
+    do_error_set(errp, err_class, NULL, NULL, fmt, ap);
+    va_end(ap);
+}
 
-    err = g_malloc0(sizeof(*err));
+static void error_set_errno_mod(Error *err, void *opaque) {
+    int os_errno = *(int *)opaque;
+    char *msg1 = err->msg;
 
-    va_start(ap, fmt);
-    msg1 = g_strdup_vprintf(fmt, ap);
     if (os_errno != 0) {
         err->msg = g_strdup_printf("%s: %s", msg1, strerror(os_errno));
         g_free(msg1);
-    } else {
-        err->msg = msg1;
-    }
-    va_end(ap);
-    err->err_class = err_class;
-
-    if (errp == &error_abort) {
-        error_report_err(err);
-        abort();
     }
+}
 
-    *errp = err;
+void error_set_errno(Error **errp, int os_errno, ErrorClass err_class,
+                     const char *fmt, ...)
+{
+    va_list ap;
 
-    errno = saved_errno;
+    va_start(ap, fmt);
+    do_error_set(errp, err_class, error_set_errno_mod, &os_errno, fmt, ap);
+    va_end(ap);
 }
 
 void error_setg_file_open(Error **errp, int os_errno, const char *filename)
@@ -93,40 +91,27 @@  void error_setg_file_open(Error **errp, int os_errno, const char *filename)
 
 #ifdef _WIN32
 
-void error_set_win32(Error **errp, int win32_err, ErrorClass err_class,
-                     const char *fmt, ...)
-{
-    Error *err;
-    char *msg1;
-    va_list ap;
+static void error_set_win32_mod(Error *err, void *opaque) {
+    int win32_err = *(int *)opaque;
+    char *msg1 = err->msg;
 
-    if (errp == NULL) {
-        return;
-    }
-    assert(*errp == NULL);
-
-    err = g_malloc0(sizeof(*err));
-
-    va_start(ap, fmt);
-    msg1 = g_strdup_vprintf(fmt, ap);
     if (win32_err != 0) {
         char *msg2 = g_win32_error_message(win32_err);
         err->msg = g_strdup_printf("%s: %s (error: %x)", msg1, msg2,
                                    (unsigned)win32_err);
         g_free(msg2);
         g_free(msg1);
-    } else {
-        err->msg = msg1;
     }
-    va_end(ap);
-    err->err_class = err_class;
+}
 
-    if (errp == &error_abort) {
-        error_report_err(err);
-        abort();
-    }
+void error_set_win32(Error **errp, int win32_err, ErrorClass err_class,
+                     const char *fmt, ...)
+{
+    va_list ap;
 
-    *errp = err;
+    va_start(ap, fmt);
+    do_error_set(errp, err_class, error_set_win32_mod, &win32_err, fmt, ap);
+    va_end(ap);
 }
 
 #endif