fix errors in virReportSystemErrorFull

* src/virterror.c (virStrerror): New function.
(virReportSystemErrorFull): Don't leak "combined".
In fact, don't even attempt allocation.
Do include the result of formatted print in final diagnostic.

diff --git a/ChangeLog b/ChangeLog
index 68e27dd625eede5201950db80a7b9a319d0961be..9aecad2121d3dd532915a483d2ada4fdd8fad669 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+Tue Jan 27 11:53:32 +0100 2009 Jim Meyering <meyering@redhat.com>
+       and Daniel P. Berrange <berrange@redhat.com>
+
+       fix errors in virReportSystemErrorFull
+       * src/virterror.c (virStrerror): New function.
+       (virReportSystemErrorFull): Don't leak "combined".
+       In fact, don't even attempt allocation.
+       Do include the result of formatted print in final diagnostic.
+
 Tue Jan 27 10:38:09 +0100 2009 Jim Meyering <meyering@redhat.com>
 
        use gnulib's stpcpy module

diff --git a/src/virterror.c b/src/virterror.c
index 0c6678151e53c77132dea246d8d5a1fe7cb947be..a577002dbd1773649025b1d0f2612787b03ae9b5 100644 (file)
--- a/src/virterror.c
+++ b/src/virterror.c
@@ -1005,57 +1005,67 @@ void virReportErrorHelper(virConnectPtr conn, int domcode, int errcode,
 
 }
 
-
-void virReportSystemErrorFull(virConnectPtr conn,
-                              int domcode,
-                              int theerrno,
-                              const char *filename ATTRIBUTE_UNUSED,
-                              const char *funcname ATTRIBUTE_UNUSED,
-                              size_t linenr ATTRIBUTE_UNUSED,
-                              const char *fmt, ...)
+static const char *virStrerror(int theerrno, char *errBuf, size_t errBufLen)
 {
-    va_list args;
-    char errorMessage[1024];
-    char systemError[1024];
-    char *theerrnostr;
-    const char *virerr;
-    char *combined = NULL;
-
 #ifdef HAVE_STRERROR_R
-#ifdef __USE_GNU
+# ifdef __USE_GNU
     /* Annoying linux specific API contract */
-    theerrnostr = strerror_r(theerrno, systemError, sizeof(systemError));
-#else
-    strerror_r(theerrno, systemError, sizeof(systemError));
-    theerrnostr = systemError;
-#endif
+    return strerror_r(theerrno, errBuf, errBufLen);
+# else
+    strerror_r(theerrno, errBuf, errBufLen);
+    return errBuf;
+# endif
 #else
     /* Mingw lacks strerror_r() and its strerror() is definitely not
      * threadsafe, so safest option is to just print the raw errno
      * value - we can at least reliably & safely look it up in the
      * header files for debug purposes
      */
-    snprintf(systemError, sizeof(systemError), "errno=%d", theerrno);
-    theerrnostr = systemError;
+    int n = snprintf(errBuf, errBufLen, "errno=%d", theerrno);
+    return (0 < n && n < errBufLen
+            ? errBuf : _("internal error: buffer too small"));
 #endif
+}
+
+void virReportSystemErrorFull(virConnectPtr conn,
+                              int domcode,
+                              int theerrno,
+                              const char *filename ATTRIBUTE_UNUSED,
+                              const char *funcname ATTRIBUTE_UNUSED,
+                              size_t linenr ATTRIBUTE_UNUSED,
+                              const char *fmt, ...)
+{
+    char strerror_buf[1024];
+    char msgDetailBuf[1024];
+
+    const char *errnoDetail = virStrerror(theerrno, strerror_buf,
+                                          sizeof(strerror_buf));
+    const char *msg = virErrorMsg(VIR_ERR_SYSTEM_ERROR, fmt);
+    const char *msgDetail = NULL;
 
     if (fmt) {
+        va_list args;
+        int n;
+
         va_start(args, fmt);
-        vsnprintf(errorMessage, sizeof(errorMessage)-1, fmt, args);
+        n = vsnprintf(msgDetailBuf, sizeof(msgDetailBuf), fmt, args);
         va_end(args);
-    } else {
-        errorMessage[0] = '\0';
+
+        size_t len = strlen (msgDetailBuf);
+        if (0 <= n && n + 2 + len < sizeof (msgDetailBuf)) {
+          char *p = msgDetailBuf + n;
+          stpcpy (stpcpy (p, ": "), errnoDetail);
+          msgDetail = msgDetailBuf;
+        }
     }
 
-    if (virAsprintf(&combined, "%s: %s", errorMessage, theerrnostr) < 0)
-        combined = theerrnostr; /* OOM, so lets just pass the strerror info as best effort */
+    if (!msgDetailBuf)
+        msgDetail = errnoDetail;
 
-    virerr = virErrorMsg(VIR_ERR_SYSTEM_ERROR, (errorMessage[0] ? errorMessage : NULL));
     virRaiseError(conn, NULL, NULL, domcode, VIR_ERR_SYSTEM_ERROR, VIR_ERR_ERROR,
-                  virerr, errorMessage, NULL, -1, -1, virerr, errorMessage);
+                  msg, msgDetail, NULL, -1, -1, msg, msgDetail);
 }
 
-
 void virReportOOMErrorFull(virConnectPtr conn,
                            int domcode,
                            const char *filename ATTRIBUTE_UNUSED,