|
 |
093822 |
From 6fcb24d785a2c2d626bac6999aee6b3ab368be15 Mon Sep 17 00:00:00 2001
|
|
|
093822 |
From: Peter Jones <pjones@redhat.com>
|
|
|
093822 |
Date: Fri, 28 Jul 2017 16:11:40 -0400
|
|
|
093822 |
Subject: [PATCH] Don't leak the last argument expanded by expandNextArg()
|
|
|
093822 |
|
|
|
093822 |
While using POPT_ARG_ARGV, I noticed this in valgrind's leak checker:
|
|
|
093822 |
|
|
|
093822 |
==1738== HEAP SUMMARY:
|
|
|
093822 |
==1738== in use at exit: 8 bytes in 1 blocks
|
|
|
093822 |
==1738== total heap usage: 94 allocs, 93 frees, 42,319 bytes allocated
|
|
|
093822 |
==1738==
|
|
|
093822 |
==1738== 8 bytes in 1 blocks are definitely lost in loss record 1 of 1
|
|
|
093822 |
==1738== at 0x4C2EB6B: malloc (vg_replace_malloc.c:299)
|
|
|
093822 |
==1738== by 0x4E3DF47: expandNextArg (popt.c:699)
|
|
|
093822 |
==1738== by 0x4E3F681: poptGetNextOpt (popt.c:1501)
|
|
|
093822 |
==1738== by 0x401F72: main (bingrep.c:433)
|
|
|
093822 |
==1738==
|
|
|
093822 |
==1738== LEAK SUMMARY:
|
|
|
093822 |
==1738== definitely lost: 8 bytes in 1 blocks
|
|
|
093822 |
==1738== indirectly lost: 0 bytes in 0 blocks
|
|
|
093822 |
==1738== possibly lost: 0 bytes in 0 blocks
|
|
|
093822 |
==1738== still reachable: 0 bytes in 0 blocks
|
|
|
093822 |
==1738== suppressed: 0 bytes in 0 blocks
|
|
|
093822 |
|
|
|
093822 |
My command line argument is a 7-byte string, and on first glance, it
|
|
|
093822 |
appears this is because both expandNextArg() and poptSaveString()
|
|
|
093822 |
duplicate the string. The copy from poptSaveString() is the consuming
|
|
|
093822 |
program's responsibility to free, but the intermediate pointer is popt's
|
|
|
093822 |
responsibility.
|
|
|
093822 |
|
|
|
093822 |
Upon further examination, it appears popt normally does free this
|
|
|
093822 |
string, but it only does it on the next entry to poptGetNextOpt(), and
|
|
|
093822 |
on cleanOSE() in the case if we're not already at the bottom of
|
|
|
093822 |
con->OptionStack.
|
|
|
093822 |
|
|
|
093822 |
This patch modifies poptResetContext() to ensure we'll always attempt to
|
|
|
093822 |
free con->os->nextArg regardless of our position in the OptionStack, and
|
|
|
093822 |
removes the duplicate free of con->os->argb in poptFreeContext(), as
|
|
|
093822 |
it's called unconditionally by the poptResetContext() call on the
|
|
|
093822 |
previous line.
|
|
|
093822 |
|
|
|
093822 |
This ensures that if poptGetNextOpt() isn't re-intered, poptFreeContext()
|
|
|
093822 |
will free the memory that was allocated. Now valgrind tells me:
|
|
|
093822 |
|
|
|
093822 |
==31734== HEAP SUMMARY:
|
|
|
093822 |
==31734== in use at exit: 0 bytes in 0 blocks
|
|
|
093822 |
==31734== total heap usage: 94 allocs, 94 frees, 42,319 bytes allocated
|
|
|
093822 |
==31734==
|
|
|
093822 |
==31734== All heap blocks were freed -- no leaks are possible
|
|
|
093822 |
|
|
|
093822 |
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
|
093822 |
---
|
|
|
093822 |
popt.c | 3 +--
|
|
|
093822 |
1 file changed, 1 insertion(+), 2 deletions(-)
|
|
|
093822 |
|
|
|
093822 |
diff --git a/popt.c b/popt.c
|
|
|
093822 |
index 1a53f40..72fbf5c 100644
|
|
|
093822 |
--- a/popt.c
|
|
|
093822 |
+++ b/popt.c
|
|
|
093822 |
@@ -230,7 +230,7 @@ void poptResetContext(poptContext con)
|
|
|
093822 |
con->os->argb = PBM_FREE(con->os->argb);
|
|
|
093822 |
con->os->currAlias = NULL;
|
|
|
093822 |
con->os->nextCharArg = NULL;
|
|
|
093822 |
- con->os->nextArg = NULL;
|
|
|
093822 |
+ con->os->nextArg = _free(con->os->nextArg);
|
|
|
093822 |
con->os->next = 1; /* skip argv[0] */
|
|
|
093822 |
|
|
|
093822 |
con->numLeftovers = 0;
|
|
|
093822 |
@@ -1617,7 +1617,6 @@ poptContext poptFreeContext(poptContext con)
|
|
|
093822 |
{
|
|
|
093822 |
if (con == NULL) return con;
|
|
|
093822 |
poptResetContext(con);
|
|
|
093822 |
- con->os->argb = _free(con->os->argb);
|
|
|
093822 |
|
|
|
093822 |
con->aliases = poptFreeItems(con->aliases, con->numAliases);
|
|
|
093822 |
con->numAliases = 0;
|
|
|
093822 |
--
|
|
|
093822 |
2.13.3
|
|
|
093822 |
|