Memory leak detected by Valgrind

[e03213ac]

This Valgrind error was generated on 3.10.8 but it also applies to the latest master branch:

==109671== 5 bytes in 1 blocks are definitely lost in loss record 1 of 6
==109671==    at 0x4C27A2E: malloc (vg_replace_malloc.c:270)
==109671==    by 0x505ACB3: DecodeNameReq (p_pmns.c:519)
==109671==    by 0x4E37247: __pmdaMainPDU (mainloop.c:232)
==109671==    by 0x4E378C7: pmdaMain (mainloop.c:428)
==109671==    by 0x402A34: main (fbit_main.c:271)

The problem is in src/libpcp_pmda/src/mainloop.c, line 232:

if ((sts = __pmDecodeChildReq(pb, &name, &subtype)) >= 0) {

The name string is not freed.

Suggested patch:

diff --git a/src/libpcp_pmda/src/mainloop.c b/src/libpcp_pmda/src/mainloop.c
index 5dff1a9..b900b72 100644
--- a/src/libpcp_pmda/src/mainloop.c
+++ b/src/libpcp_pmda/src/mainloop.c
@@ -243,6 +243,7 @@ __pmdaMainPDU(pmdaInterface *dispatch)
                /* Not INTERFACE_4 */
                sts = PM_ERR_NAME;
            }
+           free(name);
        }
        if (sts < 0)
            __pmSendError(pmda->e_outfd, FROM_ANON, sts);

[kmcdonell]

Yep, definitely a bug and the patch looks correct ... I'll push it forward [e03213ac of you want to send me your real name/email addr to kenj@kenj.com.au I'll ensure the fix is attributed correctly]

Of more interest (at least to me) is how we missed this in the PCP QA suite ... we make extensive use of valgrind to help find exactly this sort of issue, so we have a coverage problem here.

Can you share the recipe used to reproduce this?

[e03213ac]

I'm writing a PMDA that uses dynamic metrics. From what I understand this functionality is a bit uncommon, so maybe coverage is not so great? I'm sending you a simple test case derived from the "trivial" PMDA example.

[kmcdonell]

While dynamic metrics are not common, there are plenty of them already in use ...
$ pminfo -n /var/lib/pcp/pmns/root -m | grep *
kernel.percpu.interrupts PMID: 60..
proc.psinfo PMID: 3..
proc.memory PMID: 3..
proc.id PMID: 3..
proc.io PMID: 3..
proc.schedstat PMID: 3..
proc.fd PMID: 3..
proc.namespaces PMID: 3..
hotproc.psinfo PMID: 3..
hotproc.memory PMID: 3..
hotproc.id PMID: 3..
hotproc.io PMID: 3..
hotproc.schedstat PMID: 3..
hotproc.fd PMID: 3..
hotproc.namespaces PMID: 3..
sample.secret PMID: 29..
sampledso.secret PMID: 30..
mmv PMID: 70..
postgresql PMID: 110..

[kmcdonell]

cut-n-paste ate my asterisks in the last comment ... let me try again ...
$ pminfo -n /var/lib/pcp/pmns/root -m | grep *
kernel.percpu.interrupts PMID: 60..
proc.psinfo PMID: 3..
proc.memory PMID: 3..
proc.id PMID: 3..
proc.io PMID: 3..
proc.schedstat PMID: 3..
proc.fd PMID: 3..
proc.namespaces PMID: 3..
hotproc.psinfo PMID: 3..
hotproc.memory PMID: 3..
hotproc.id PMID: 3..
hotproc.io PMID: 3..
hotproc.schedstat PMID: 3..
hotproc.fd PMID: 3..
hotproc.namespaces PMID: 3..
sample.secret PMID: 29..
sampledso.secret PMID: 30..
mmv PMID: 70..
postgresql PMID: 110..

[kmcdonell]

Fixed in commit a6cb593

Thanks Emanuele.