From: Jim Meyering <meyering@redhat.com>
Date: Tue, 27 Jan 2009 10:57:18 +0000 (+0000)
Subject: fix errors in virReportSystemErrorFull
X-Git-Tag: LIBVIRT_0_6_0~45
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e15147c14117333e780ef1b3d793fcb0dffa4607;p=thirdparty%2Flibvirt.git

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 68e27dd625..9aecad2121 100644
--- 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 0c6678151e..a577002dbd 100644
--- 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,