RPM Community Forums

Mailing List Message of <popt-devel>

Re: POPT's API has designed in memory leaks. What to do?

From: Jeff Johnson <n3npq@mac.com>
Date: Thu 17 Jun 2010 - 07:38:12 CEST
Message-id: <FA32509A-C949-4286-99BB-BDBE06D928E7@mac.com>

On Jun 16, 2010, at 4:59 PM, Markdv wrote:

> Jeff Johnson wrote:
>> I happen to have a valgrind trace on my screen that shows the issue
>> ==25160== ==25160== 49 bytes in 1 blocks are still reachable in loss record 2 of 2
>> ==25160==    at 0x4005BDC: malloc (vg_replace_malloc.c:195)
>> ==25160==    by 0x314D9A: poptGetNextOpt (popt.c:1203)
>> ==25160==    by 0x40697CD: rpmcliInit (poptALL.c:751)
>> ==25160==    by 0x804AC45: main (rpmqv.c:385)
>> ==25160== The "memory leak" is actually a POPT design feature. Every string
>> returned from POPT is malloc'd so that an application
>> can do whatever it wishes with the string without
>> worrying about side effects of fiddling with the memory.
>> Unfortunately, POPT is mostly not used correctly, and the expectation is
>> 	Hey POPT handles argv strings, I shouldn't _HAVE_ to manage those!?!
> 
> I don't mind having to manage those, as long as it is documented.
> 
> I just started using libpopt for the very first time today and checked my code with valgrind which showed a leak. The minimal program below always leaks an amount of memory equal to the length of the argument of the "hostname" option +1. Despite the fact that I think I'm free-ing everything I'm supposed to free... After skimming the libpopt source I inserted the two commented lines that, when uncommented (duh), seem to fix the leak... lucky shot I guess. :)
> 
> Does this not demonstrate that there actually is a memory leak in popt?

I use popt & valgrind daily in all sorts of configurations.

So no, it does _NOT_ demonstrate that there's a leak in popt.

The whole point of my e-mail is/was that the arg is malloc'd by
intent so that the application can do whatever it wishes
with the returned memory.

And its not sufficient to document the behavior (which has been this
way for a decade). Inquiring minds want to know even if the
amount of memory is <100b.

> 
> If not, I would appreciate someone explaining how I'm using libpopt wrongly.
> 
> #include <stdio.h>
> #include <stdlib.h>
> #include <popt.h>
> 
> /** #include "/tmp/popt-1.14/poptint.h" **/
> 

You should not ever need/want to include "poptint.h" which
contains routines and data structures internal to popt.

> struct opts {
>    char    *hostname;
>    int     verbose;
> } opts;
> 
> struct poptOption optTable[] = {
>    {"hostname", 'h', POPT_ARG_STRING, &opts.hostname,
>        0, "hostname", "HOST" },
>    {"verbose",  'v', POPT_ARG_NONE,   &opts.verbose,
>        0, "be verbose", 0 },
>    POPT_AUTOHELP
>    POPT_TABLEEND
> };
> 
> int main( int argc, const char **argv )
> {
> 
>    poptContext optCon;
> 
>    optCon = poptGetContext(NULL, argc, argv,
>                            optTable, POPT_CONTEXT_POSIXMEHARDER);
> 

Do you _REALLY_ want/need POSIXMEHARDER? The issue is
largely whether options must strictly proceed args or not.

IIRC (I don't use anything but popt for option processing)
getopt_long() default behavior in linux is _NOT_ POSIXly correct
but rather the more convenient process --foo where ever found.

>    poptGetNextOpt(optCon);
> 

There's usually a loop over poptGetNextOpt. E.g. here's what is in RPM

    /* Process all options, whine if unknown. */
    while ((rc = poptGetNextOpt(optCon)) > 0) {
        const char * optArg = poptGetOptArg(optCon);
        optArg = _free(optArg);
        switch (rc) {
        default:
            fprintf(stderr, _("%s: option table misconfigured (%d)\n"),
                __progname, rc);
            exit(EXIT_FAILURE);

            /*@notreached@*/ /*@switchbreak@*/ break;
        }
    }

The free in the loop is what is needed for "squeaky clean".

But truly, I wouldn't fuss too much about 10-20 bytes of "leak".
glibc leaks far more than that, but the leaks are masked in valgrind.

And technically none of these are "leaks" because its one-time
allocation, not continually increasing unfree'd memory.

>    printf("Options:\n"
>           "hostname: %s\n"
>           "verbose: %d\n",
>           opts.hostname,
>           opts.verbose);
> 
>    free(opts.hostname);
> /**    free(optCon->os->nextArg);  **/
> 
>    poptFreeContext(optCon);
> 

See if the loop fixes, then try using poptGetOptArg()
and free the result.

Whether that is obvious or convenient or correct or documented is a different matter.

But whatever behavior is implemented in POPT has been largely unchanged this century.


>    return 0;
> }
> 
> 
> Regards,
> Mark.
> 
>> I get a tedious bug report every couple of months from otherwise honest
>> attempts to use valgrind for application "squeaky clean" memory auditing.
>> Should this behavior be changed in POPT 2.0? It's a 1-liner change to remove
>> a strdup() somewhere, but the change does have profound (but minor, who
>> actually cares about a 49b 1-time memory leak these days) ramifications.
>> Meanwhile I'm way tired of explaining why its _NOT_ a memory leak, but rather
>> buggy use of POPT.
>> Opinions welcomed.
>> 73 de Jeff
> 
> ______________________________________________________________________
> POPT Library                                           http://rpm5.org
> Developer Communication List                       popt-devel@rpm5.org
Received on Thu Jun 17 07:39:09 2010
Driven by Jeff Johnson and the RPM project team.
Hosted by OpenPKG and Ralf S. Engelschall.
Powered by FreeBSD and OpenPKG.