[libvirt] [PATCH] fix errors in virReportSystemErrorFull

[Jim Meyering]

"Daniel P. Berrange" <berrange at redhat.com> wrote:
> On Tue, Jan 27, 2009 at 11:28:32AM +0100, Jim Meyering wrote:
>> "Daniel P. Berrange" <berrange at redhat.com> wrote:
>> ...
>> > Looking at the whole method again, I think it needs to be re-written to
>> > something closer to this:
>>
>> Ok, I've adapted that.
>> +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;
>
> This should be   if (!msgDetail) - indeed just noticed the compiler
> warns on this
>
> cc1: warnings being treated as errors
> virterror.c: In function 'virReportSystemErrorFull':
> virterror.c:1062: error: the address of 'msgDetailBuf' will always evaluate as 'true'

Good catch.
I've just committed this:

>From 2bb6830c061a580328abf4647a26b9ecc7f7eaa1 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Tue, 27 Jan 2009 13:25:16 +0100
Subject: [PATCH] virterror.c: don't read beyond end of buffer upon OOM

* src/virterror.c (virReportSystemErrorFull): Fix typo in
my previous change.  Patch by Daniel P. Berrange.
---
 src/virterror.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/virterror.c b/src/virterror.c
index a577002..160fea3 100644
--- a/src/virterror.c
+++ b/src/virterror.c
@@ -1059,7 +1059,7 @@ void virReportSystemErrorFull(virConnectPtr conn,
         }
     }

-    if (!msgDetailBuf)
+    if (!msgDetail)
         msgDetail = errnoDetail;

     virRaiseError(conn, NULL, NULL, domcode, VIR_ERR_SYSTEM_ERROR, VIR_ERR_ERROR,
--
1.6.1.401.gf39d5