"Daniel P. Berrange" <berrange@redhat.com> wrote:
On Tue, Jan 27, 2009 at 11:28:32AM +0100, Jim Meyering wrote:
"Daniel P. Berrange" <berrange@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@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