[libvirt] [PATCH 0/2] Couple of logging memleaks
I've found them by pure chance and they were easy to fix. Michal Privoznik (2): virLogHostnameString: Don't leak hostname virLogVMessage: Don't leak rawinitmsg src/util/virlog.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) -- 2.4.10
Once @hostname is printed into @hoststr we don't need it anymore. ==6879== 5 bytes in 1 blocks are definitely lost in loss record 10 of 1,064 ==6879== at 0x4C29F80: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==6879== by 0xA7ED599: strdup (in /lib64/libc-2.21.so) ==6879== by 0x552C126: virStrdup (virstring.c:726) ==6879== by 0x553B13E: virGetHostnameImpl (virutil.c:720) ==6879== by 0x553B1BF: virGetHostnameQuiet (virutil.c:741) ==6879== by 0x54FA3FD: virLogHostnameString (virlog.c:462) ==6879== by 0x54FAB0F: virLogVMessage (virlog.c:645) ==6879== by 0x54FA680: virLogMessage (virlog.c:531) ==6879== by 0x54FBBF4: virLogParseOutputs (virlog.c:1130) ==6879== by 0x11CB4F: daemonSetupLogging (libvirtd.c:685) ==6879== by 0x11E137: main (libvirtd.c:1297) Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virlog.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/util/virlog.c b/src/util/virlog.c index 7d5a266..ecb1024 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -469,6 +469,7 @@ virLogHostnameString(const char **rawmsg, VIR_FREE(hostname); return -1; } + VIR_FREE(hostname); if (virLogFormatString(msg, 0, NULL, VIR_LOG_INFO, hoststr) < 0) { VIR_FREE(hoststr); -- 2.4.10
On 01/06/2016 11:19 AM, Michal Privoznik wrote:
Once @hostname is printed into @hoststr we don't need it anymore.
==6879== 5 bytes in 1 blocks are definitely lost in loss record 10 of 1,064 ==6879== at 0x4C29F80: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==6879== by 0xA7ED599: strdup (in /lib64/libc-2.21.so) ==6879== by 0x552C126: virStrdup (virstring.c:726) ==6879== by 0x553B13E: virGetHostnameImpl (virutil.c:720) ==6879== by 0x553B1BF: virGetHostnameQuiet (virutil.c:741) ==6879== by 0x54FA3FD: virLogHostnameString (virlog.c:462) ==6879== by 0x54FAB0F: virLogVMessage (virlog.c:645) ==6879== by 0x54FA680: virLogMessage (virlog.c:531) ==6879== by 0x54FBBF4: virLogParseOutputs (virlog.c:1130) ==6879== by 0x11CB4F: daemonSetupLogging (libvirtd.c:685) ==6879== by 0x11E137: main (libvirtd.c:1297)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virlog.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/src/util/virlog.c b/src/util/virlog.c index 7d5a266..ecb1024 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -469,6 +469,7 @@ virLogHostnameString(const char **rawmsg, VIR_FREE(hostname); return -1; } + VIR_FREE(hostname);
if (virLogFormatString(msg, 0, NULL, VIR_LOG_INFO, hoststr) < 0) { VIR_FREE(hoststr); ACK.
Instead of misusing a const string to hold up runtime allocated data, introduce new variable @hoststr and obey const correctness. ==6879== 15 bytes in 1 blocks are definitely lost in loss record 68 of 1,064 ==6879== at 0x4C29F80: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==6879== by 0xA7DDF97: vasprintf (in /lib64/libc-2.21.so) ==6879== by 0x552BBC6: virVasprintfInternal (virstring.c:493) ==6879== by 0x552BCDB: virAsprintfInternal (virstring.c:514) ==6879== by 0x54FA44C: virLogHostnameString (virlog.c:468) ==6879== by 0x54FAB0F: virLogVMessage (virlog.c:645) ==6879== by 0x54FA680: virLogMessage (virlog.c:531) ==6879== by 0x54FBBF4: virLogParseOutputs (virlog.c:1130) ==6879== by 0x11CB4F: daemonSetupLogging (libvirtd.c:685) ==6879== by 0x11E137: main (libvirtd.c:1297) Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virlog.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/util/virlog.c b/src/util/virlog.c index ecb1024..b8398d1 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -456,7 +456,7 @@ virLogVersionString(const char **rawmsg, * reporting APIs or logging APIs, to prevent recursion */ static int -virLogHostnameString(const char **rawmsg, +virLogHostnameString(char **rawmsg, char **msg) { char *hostname = virGetHostnameQuiet(); @@ -612,6 +612,7 @@ virLogVMessage(virLogSourcePtr source, if (priority >= virLogOutputs[i].priority) { if (virLogOutputs[i].logInitMessage) { const char *rawinitmsg; + char *hoststr = NULL; char *initmsg = NULL; if (virLogVersionString(&rawinitmsg, &initmsg) >= 0) virLogOutputs[i].f(&virLogSelf, VIR_LOG_INFO, @@ -619,11 +620,12 @@ virLogVMessage(virLogSourcePtr source, timestamp, NULL, 0, rawinitmsg, initmsg, virLogOutputs[i].data); VIR_FREE(initmsg); - if (virLogHostnameString(&rawinitmsg, &initmsg) >= 0) + if (virLogHostnameString(&hoststr, &initmsg) >= 0) virLogOutputs[i].f(&virLogSelf, VIR_LOG_INFO, __FILE__, __LINE__, __func__, - timestamp, NULL, 0, rawinitmsg, initmsg, + timestamp, NULL, 0, hoststr, initmsg, virLogOutputs[i].data); + VIR_FREE(hoststr); VIR_FREE(initmsg); virLogOutputs[i].logInitMessage = false; } @@ -636,6 +638,7 @@ virLogVMessage(virLogSourcePtr source, if (virLogNbOutputs == 0) { if (logInitMessageStderr) { const char *rawinitmsg; + char *hoststr = NULL; char *initmsg = NULL; if (virLogVersionString(&rawinitmsg, &initmsg) >= 0) virLogOutputToFd(&virLogSelf, VIR_LOG_INFO, @@ -643,11 +646,12 @@ virLogVMessage(virLogSourcePtr source, timestamp, NULL, 0, rawinitmsg, initmsg, (void *) STDERR_FILENO); VIR_FREE(initmsg); - if (virLogHostnameString(&rawinitmsg, &initmsg) >= 0) + if (virLogHostnameString(&hoststr, &initmsg) >= 0) virLogOutputToFd(&virLogSelf, VIR_LOG_INFO, __FILE__, __LINE__, __func__, - timestamp, NULL, 0, rawinitmsg, initmsg, + timestamp, NULL, 0, hoststr, initmsg, (void *) STDERR_FILENO); + VIR_FREE(hoststr); VIR_FREE(initmsg); logInitMessageStderr = false; } -- 2.4.10
On 01/06/2016 11:19 AM, Michal Privoznik wrote:
Instead of misusing a const string to hold up runtime allocated data, introduce new variable @hoststr and obey const correctness.
==6879== 15 bytes in 1 blocks are definitely lost in loss record 68 of 1,064 ==6879== at 0x4C29F80: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==6879== by 0xA7DDF97: vasprintf (in /lib64/libc-2.21.so) ==6879== by 0x552BBC6: virVasprintfInternal (virstring.c:493) ==6879== by 0x552BCDB: virAsprintfInternal (virstring.c:514) ==6879== by 0x54FA44C: virLogHostnameString (virlog.c:468) ==6879== by 0x54FAB0F: virLogVMessage (virlog.c:645) ==6879== by 0x54FA680: virLogMessage (virlog.c:531) ==6879== by 0x54FBBF4: virLogParseOutputs (virlog.c:1130) ==6879== by 0x11CB4F: daemonSetupLogging (libvirtd.c:685) ==6879== by 0x11E137: main (libvirtd.c:1297)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virlog.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/src/util/virlog.c b/src/util/virlog.c index ecb1024..b8398d1 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -456,7 +456,7 @@ virLogVersionString(const char **rawmsg, * reporting APIs or logging APIs, to prevent recursion */ static int -virLogHostnameString(const char **rawmsg, +virLogHostnameString(char **rawmsg, char **msg) { char *hostname = virGetHostnameQuiet(); @@ -612,6 +612,7 @@ virLogVMessage(virLogSourcePtr source, if (priority >= virLogOutputs[i].priority) { if (virLogOutputs[i].logInitMessage) { const char *rawinitmsg; + char *hoststr = NULL; char *initmsg = NULL; if (virLogVersionString(&rawinitmsg, &initmsg) >= 0) virLogOutputs[i].f(&virLogSelf, VIR_LOG_INFO, @@ -619,11 +620,12 @@ virLogVMessage(virLogSourcePtr source, timestamp, NULL, 0, rawinitmsg, initmsg, virLogOutputs[i].data); VIR_FREE(initmsg); - if (virLogHostnameString(&rawinitmsg, &initmsg) >= 0) + if (virLogHostnameString(&hoststr, &initmsg) >= 0) virLogOutputs[i].f(&virLogSelf, VIR_LOG_INFO, __FILE__, __LINE__, __func__, - timestamp, NULL, 0, rawinitmsg, initmsg, + timestamp, NULL, 0, hoststr, initmsg, virLogOutputs[i].data); + VIR_FREE(hoststr); VIR_FREE(initmsg); virLogOutputs[i].logInitMessage = false; } @@ -636,6 +638,7 @@ virLogVMessage(virLogSourcePtr source, if (virLogNbOutputs == 0) { if (logInitMessageStderr) { const char *rawinitmsg; + char *hoststr = NULL; char *initmsg = NULL; if (virLogVersionString(&rawinitmsg, &initmsg) >= 0) virLogOutputToFd(&virLogSelf, VIR_LOG_INFO, @@ -643,11 +646,12 @@ virLogVMessage(virLogSourcePtr source, timestamp, NULL, 0, rawinitmsg, initmsg, (void *) STDERR_FILENO); VIR_FREE(initmsg); - if (virLogHostnameString(&rawinitmsg, &initmsg) >= 0) + if (virLogHostnameString(&hoststr, &initmsg) >= 0) virLogOutputToFd(&virLogSelf, VIR_LOG_INFO, __FILE__, __LINE__, __func__, - timestamp, NULL, 0, rawinitmsg, initmsg, + timestamp, NULL, 0, hoststr, initmsg, (void *) STDERR_FILENO); + VIR_FREE(hoststr); VIR_FREE(initmsg); logInitMessageStderr = false; }
ACK.
participants (2)
-
Laine Stump -
Michal Privoznik