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 - 20:00:21 CEST
Message-ID: <4C0D33B5.3040909@dannysung.com>
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).

But yeah, I guess I'd opt for consistency as well.  The thing that 
surprised me was that while poptGetOptArg() expects the caller to free 
it, it's still returning a const.  I use 'const' not as memory 
protection (we know it doesn't do that), but as a signifier that the 
caller doesn't need to worry about freeing the contents.  But whatever, 
I just put a /* popt requires this to be freed */ comment every time I 
use it.

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).

If all Get's in popt allocate, that's fine (I actually hadn't noticed 
that because I wasn't really expecting it).  But if there's ever a time 
when you have a mix, I'd also suggest function names containing "New" 
(or "Dup" or "Alloc" or something of the sort) to drive home the point 
that something's being allocated.  "Get" to me typically means retrieve, 
not allocate.  But like I said, it's not a huge issue.  Just add the 
poptFreeOptArg() and I think it'll be clear for everyone.

On 6/7/2010 9:02 AM, Jeff Johnson wrote:
> On Jun 7, 2010, at 11:50 AM, Mark Hatle wrote:
>> Jeff Johnson wrote:
>>> On Jun 7, 2010, at 10:37 AM, Mark Hatle wrote:
>>>> The way I've usually addressed this is to either add an option to the library that changes the default behavior from strdup to passing the address with the expectation of const.
>>> I'd rather _NOT_ go the "Have it your own way!" route in
>>> a API/ABI becuase it adds yet another datum that needs to
>>> be extracted from lusers doing POPT support.
>> Runtime not build time.  For a library like popt, build time would be really bad.
> I was thinking "run-time" ...
>> For runtime, it should be a simple 'switch' for the strdup call.
> ... and there's nothing "simple" about insturmenting choice in POPT
> underneath applications.
> Its either Yet Another Arcane flag that noone uses or a goosey-loosey
> envvar that makes debugging run-time dependent, see the tedium of
> POSIX_ME_HARDER that is already in POPT since forever.
> But sure I know the "Have it your own way!" run-time drill quite well too.
> Just I'm hoping not to go there, I do like KISSy-poo.
>>>> Either by adding const style prototypes/functions, or by using some mechanism to change the behavior of all of the functions.
>>> (aside)
>>> Well I've gone multiple ways with the C borkage of "const char **"
>>> vs "char *const *" in POPT and RPM. These days there's so much spewage
>>> from GCC that I don't believe that compilers or language hints solve
>>> any "real world" issue. But I can/will go to PROT_READ mandatory hardware
>>> enforcement using mmap(2) if that is what is desired. Its easier
>>> to implement the code than it is to discuss the various religion's
>>> coding fetishism's Yet Agin. But I digress ...
>>> Note that it _WAS_ GCC's writable strings that forced the malloc
>>> into POPT in the first place, where POPT sometimes returned
>>> strings from RO memory, and sometimes from argv, and the morons were confused.
>>> Adding the strdup() to POPT was 1 less issue to worry about. Another digression ...
>> Ya, I don't doubt it.  I believe the issue comes down to consistency.  Either everything popt returns should be duplicated so that the user has to clear them, or nothing should be so the user knows that it's expected to be read-only. (Enforcing read-only is another issue entirely, and one that I don't really think is popt's responsibility.  Set the prototype properly on the functions and if the user violates them so be it.. they live with the consequences.)
> What is currently implemented is the former, malloc everything returned,
> and those with clue will figger it out. That's general, just surprising.
> But I can/will make the audit's easier if that is what is desired. Its
> a bleeping string, deal with it, stop fussing POPT "leaks" is MNSHO privately.
>>>> My biggest concern is the potential retrofit of existing apps that expect the current behavior.. but I agree with many of the submitters.. popt really should be sending const points and then the app needs to strdup.
>>> And so "legacy compatibility" again again again. POPT 2.0 is a major change
>>> with no "compatibility" implied or intended. Meanwhile I will strive to
>>> make the change as painless as possible. But there are issues (aka "deep hacks")
>>> that have been in POPT for a decade that aren't the right thing to do. And even
>>> though I've overloaded just about everything possible in the 7-tuple of a
>>> POPT table item, there are these issues:
>> If both popt 1 and popt 2 can live on the same machine and have different sonames, then I vote for no strdup.. it's the users problem to duplicate what they want.
> Changing the soname or even (gasp!) using ELF symbol versioning
> is quite doable, all the necessary precursor elements have been in place
> for years.
>> IMHO libraries should not protect the user from themselves.  If they are dumb enough to modify memory or expect that memory will live beyond a certain pre-defined lifecycle.. then it's their problem to fix...  (Note of course reentrancy and such could be an issue.. but thats only a problem if it's allowed by popt.)
> Hehe, so PROT_READ LART'ing is where you want to go ;-) That would
> be fine with me except I'm also trying to avoid any POPT "support" headaches.
> Note that C macros could be used to ease the pain of "long"<->  "void *"
> punning. But I _REALLY_ need another pointer field somehow without overloading
> the two i18n --help fields which are already way way way too complicated
> for my taste.
> 73 de Jeff
> ______________________________________________________________________
> POPT Library                                           http://rpm5.org
> Developer Communication List                       popt-devel@rpm5.org

Please note my new email address: danny@dannysung.com
Received on Mon Jun 7 20:07:30 2010
Driven by Jeff Johnson and the RPM project team.
Hosted by OpenPKG and Ralf S. Engelschall.
Powered by FreeBSD and OpenPKG.