Re: [libvirt] [PATCH] logging: fix off-by-one bug

Valgrind caught that our log wrap-around was going 1 past the end. Regression introduced in commit b16f47a; previously the buffer was static and size+1 bytes, but now it is dynamic and exactly size bytes. * src/util/logging.c (virLogStr): Don't write past end of log. --- An alternative would be to malloc one larger; but since the log is likely to be a page size multiple and large enough to be worth malloc using mmap, going one larger is likely to waste the bulk of a page. Also, I like always NUL-terminating the current end of the log (without including that NUL in the log length). src/util/logging.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/logging.c b/src/util/logging.c index b972f8a..f4910ad 100644 --- a/src/util/logging.c +++ b/src/util/logging.c @@ -326,7 +326,7 @@ static void virLogStr(const char *str, int len) { return; if (len <= 0) len = strlen(str); - if (len > virLogSize) + if (len >= virLogSize) return; virLogLock(); @@ -336,13 +336,13 @@ static void virLogStr(const char *str, int len) { if (virLogEnd + len >= virLogSize) { tmp = virLogSize - virLogEnd; memcpy(&virLogBuffer[virLogEnd], str, tmp); - virLogBuffer[virLogSize] = 0; memcpy(&virLogBuffer[0], &str[tmp], len - tmp); virLogEnd = len - tmp; } else { memcpy(&virLogBuffer[virLogEnd], str, len); virLogEnd += len; } + virLogBuffer[virLogEnd] = 0; /* * Update the log length, and if full move the start index */