RPM Community Forums

Mailing List Message of <popt-devel>

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

From: Danny Sung <danny@dannysung.com>
Date: Mon 07 Jun 2010 - 22:16:27 CEST
Message-ID: <4C0D539B.7000404@dannysung.com>


On 6/7/2010 11:38 AM, Jeff Johnson wrote:
>
> On Jun 7, 2010, at 2:00 PM, Danny Sung wrote:
>
>> My personal choice in things I write is to expect the caller to strdup().  But I understand the reentrancy issue here.  (though why you'd be using popt in a thread is beyond me at this time.. and it does have a poptContext handle anyway).
>>
>
> popt does not use threads, applications do. so its applications, not popt,
> that force -lpopt into multi-threaded environments. All that POPT can
> do is to try and work transparently in both environments.

Yeah, I meant as an app developer... I can't see why you'd want to use 
popt outside of a single thread.  That's the only context where 
reentrancy would be an issue while using the popt library, no?


>> If this is going to continue to be the behavior, I'd suggest a poptFreeOptArg() call just to highlight the necessity (and deal with type and NULL pointer checks).
>>
>
> Why add a new method for what is already a well known operation called free(3)?
>
> I mean I can add poptFreeArg in 1 #define to popt.h, but why bother?

Because it's not just "free()"   It's:
    if(optarg) free((void *)optarg);

Not a huge deal, but a bit of an inconvenience.  Plus like I said the 
added function just higlights the need to free the return from the Get 
call.  I actually have a str_delete() call in my standard library that 
encapsulates the if.  But I still have to call it like: str_delete((char 
*)optarg) just to break the const to satisfy the compiler warnings.


> AFAIK, all POPT args are either returned by value (like POPT_ARG_INT) or
> through malloc'd memory _ALWAYS_. The rules on table callbacks are different
> than the rules for the no-brainer, in-line loop based getopt(3) like processing.

Yeah, it's fine.  As I said it was just a little unexpected when I first 
started using popt.  I didn't realize it until I ran a memory checker. 
And from the 'const' type in the Get call I had thought it was a bug in 
popt.

Looking over the man page right now (popt 1.16), I don't really see 
where it talks about freeing this memory either.  It's compounded by the 
fact that there is a poptFreeContext and poptDupArg() calls.  Those both 
imply to me that popt is doing memory management for me.

There's also a poptPeekArg() call.  Is the app developer expected to 
free that one as well?  If so, then the example seems to lead to a 
memory leak as well:

           portname = poptGetArg(optCon);
           if((portname == NULL) || !(poptPeekArg(optCon) == NULL))
              usage(optCon, 1, "Specify a single port", ".e.g., /dev/cua0");

Neither the return from poptGetArg() or poptPeekArg() are being freed.

If this is the behavior you want, I'm fine with it.  But I think the man 
page needs updating.  (Apologies if it's already been updated, but I 
don't see it on my systems).

If you'd like, I'll be happy to contribute these changes to the man 
page.  It just may take me a weeks before I have any free time.
Received on Mon Jun 7 22:23:41 2010
Driven by Jeff Johnson and the RPM project team.
Hosted by OpenPKG and Ralf S. Engelschall.
Powered by FreeBSD and OpenPKG.