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

[Seonkon Choi]

On Thu, Jun 17, 2010 at 10:09:37AM -0400, Jeff Johnson wrote:
> 
> On Jun 17, 2010, at 4:27 AM, Markdv wrote:
> 
> > 
> > I just don't see why you wouldn't want to "fix" this. Seems like all you'd 
> > have to do is add
> > 
> >con->os->nextArg = _free(con->os->nextArg);
> > 
> > to poptFreeContext(poptContext con) and be done with it.
> > 
> 
> There's a double free with your suggested "fix" if/when the application
> has also free'd the memory returned.
> 
> 73 de Jeff
> 

Sorry Jeff,
could you please show me an example of double-free against with that "fix" ?

For the poptGetOptArg(), you mentioned,
~snip~
1554 char * poptGetOptArg(poptContext con)
1555 {
1556 char * ret = NULL;
1557 if (con) {
1558 ret = con->os->nextArg;
1559 con->os->nextArg = NULL;
1560 }
1561 return ret;
1562 }
~snip~
So if someone called the poptGetOptArg(), _free(con->os->nextArg) of the 
poptResetContext() will do nothing.

I know, this thread is too old.
But it was talking about popt >= 1.16 and people still are using the popt-1.16.
So it is not too old to continue.
__
POPT Library   http://rpm5.org
Developer Communication List   popt-devel@rpm5.org

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

[Jeff Johnson]

On Jun 17, 2010, at 4:27 AM, Markdv wrote:

> 
> I just don't see why you wouldn't want to "fix" this. Seems like all you'd 
> have to do is add
> 
>con->os->nextArg = _free(con->os->nextArg);
> 
> to poptFreeContext(poptContext con) and be done with it.
> 

There's a double free with your suggested "fix" if/when the application
has also free'd the memory returned.

73 de Jeff

__
POPT Library   http://rpm5.org
Developer Communication List   popt-devel@rpm5.org

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

[Jeff Johnson]

> 
> What? A bug - albeit not a very serious one - that's been there for ever is, 
> therefore, not a bug?
> 

That isn't what I said, in fact quite the opposite.

73 de Jeff
__
POPT Library   http://rpm5.org
Developer Communication List   popt-devel@rpm5.org

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

[Markdv]

Jeff Johnson wrote:

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.


Clear. And I think that the "leak" I demonstrated is not the same one 
you are talking about. Sorry for confusing the issue.



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.


Misunderstood you, thought you meant something else. Never mind.


If not, I would appreciate someone explaining how I'm using libpopt wrongly.

#include 
#include 
#include 

/** #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.


Well DUH!, I only did this in this case in order to be able to access 
the structure member that I thought should have been freed. Just in 
order to demonstrate the "problem".



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.


Yes, perhaps, but beside the point imho.


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


Yes, but there doesn't have to be a loop. At least not according to the 
documentation, from popt(3):


   If all of the command-line options are  handled  through  arg
   pointers,  command-line  parsing  is reduced to the following
   line of code:

   rc = poptGetNextOpt(poptcon);

Which is exactly what I'm doing...


/* 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".


What's the big difference? Instead of manually getting the optArg in the 
loop I have them assigned to variables via "arg pointers". And I _do_ 
free that memory after I'm done with it.


So (afaict) I'm using libpopt in a documented way, and freeing 
everything I can and am supposed to free, and still it "leaks".



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.


I agree completely, aside from the fact that valgrind output does not 
show the program as "squeaky clean" it's absolutely nothing to really 
worry about. But having it show as clean instead of showing some 
possible problem that I have to l

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

[Jeff Johnson]

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 
> #include 
> #include 
> 
> /** #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
> 
> __

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

[Markdv]

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?

If not, I would appreciate someone explaining how I'm using libpopt wrongly.

#include 
#include 
#include 

/** #include "/tmp/popt-1.14/poptint.h" **/

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

poptGetNextOpt(optCon);

printf("Options:\n"
   "hostname: %s\n"
   "verbose: %d\n",
   opts.hostname,
   opts.verbose);

free(opts.hostname);
/**free(optCon->os->nextArg);  **/

poptFreeContext(optCon);

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

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

[Jeff Johnson]

On Jun 7, 2010, at 2:38 PM, Jeff Johnson wrote:
> 
> 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.
> 
> OTOH, some of the code paths in POPT are quite twisty, perhaps there's 
> someplace
> where another strdup/malloc is needed.
> 

Here is poptPeekArg() (I happen to have on my screen):

const char * poptPeekArg(poptContext con)
{
const char * ret = NULL;
if (con && con->leftovers != NULL && con->nextLeftover < con->numLeftovers)
ret = con->leftovers[con->nextLeftover];
return ret;
}

So no malloc with poptPeekArg() returns atm, largely for hysterical
reasons.

Its easy enough to add strdup (or not) to poptPeekArg(). What isn't clear is
whether a consistent rule like
All memory is malloc'd before return.
is preferred to the mysteriously inconsistent current behavior in POPT.

73 de Jeff
__
POPT Library   http://rpm5.org
Developer Communication List   popt-devel@rpm5.org

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

[Jeff Johnson]

On Jun 7, 2010, at 4:40 PM, Danny Sung wrote:

> 
> On 6/7/2010 11:29 AM, Jeff Johnson wrote:
>> 
>> On Jun 7, 2010, at 2:00 PM, Danny Sung wrote:
>> 
>>> I agree.  I have no problems with breaking binary&source compatibility 
>>> especially if that change will make popt better/easier in the future. But a 
>>> change in name seems necessary for app developers to make that choice.  eg. 
>>> what happens if you're building mulitiple different software packages... 
>>> both dynamically linking popt, but one uses 1.x and the other 2.x?  The 
>>> header files would also need something like #include  or 
>>> something like that.  Can't say I particularly like it... but it does those 
>>> issues somewhat cleanly.
>>> 
>> 
>> I respectfully and strongly _DISAGREE_. The issue can be best be
>> dealt with by using ELF symbol versioning (once I get that it place
>> again in popt-1.17) where a single library can provide _BOTH_
>> POPT 1.0 and POPT 2.0 functionality _TRANSPARENTLY_ (one the
>> necessary pre-requsite of adding ELF symbol versioning is achieved).
> 
> Doesn't this only work if you're maintaining source compatibility? (whiihc is 
> good, I just thought you said you weren't aiming for that).
> Sorry, not trying to be difficult... just trying to understand.  Also does 
> ELF symbol versioning work for both dynamically and statically linked apps?
> 

ELF symbol versioning is what glibc uses to have a forward looking
upgrade path. If it "works" for glibc, that's all I (and perhaps you) need to 
know.

I do know more about ELF symbol versioning, its not very hard, if interested.
Its largely a conscious design choice releasing a library, the rest is
just "stuff".

>> If I rename "popt" to "tdod" (or "popt2" that precludes even the attempt at
>> doing better engineering using ELF symbol versioning. While I'm
>> not averse to renaming, I've seen the bleary package churn from
>> libxml ->  libxml2 and GNOME 1.0 ->  GNOME 2.0. No way Jose!
> 
> Yeah, I thought it was ugly too.  But it makes sense... in an ugly way.

We agree "pugly". FWIW, months of my life has been spent staring
at splint traces. If you think the code is ugly, wait until you see
10K lines of spewage (and attempt to "fix").

These days scan.coverity.com is better technology than splint (and
POPT is already a registered project with scan.coverity.com, I signed
up almost instantly when scan.coverity.com was announced).

However, scan.coverity.com takes quite some time (months and years)
to scan a new release, and is an exorbitanly prohibitively expensive
proprietary tool for FLOSS development.

Note that scan.coverity.com still thinks that rpm-2.5.x released
in August, 1998 is still "current" software that is usefully scanned
(because rpm-2.5.x is _STILL_ in NetBSD 12 years later). MHO differs
considerably, I have no need to examine "bugs" in 12 year old releases
for any purpose whatsoever. But I digress ...


Which makes splint attractive for POPT development even if the annotations
are quite pugly and intrusive.

hth

73 de Jeff
> __
> POPT Library   http://rpm5.org
> Developer Communication List   popt-devel@rpm5.org

__
POPT Library   http://rpm5.org
Developer Communication List   popt-devel@rpm5.org

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

[Jeff Johnson]

On Jun 7, 2010, at 4:16 PM, Danny Sung wrote:

> 
> 
> 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?
> 

Ah ...

There's lots of usage cases for popt arg processing. E.g. RPM
has multiple embeddings and is just starting to explore the mysteries
of multi-threaded development (its taken most of the last year to
get posix mutexes stabilized and attached transparently to ~40 RPM objects)

POPT is used for all option processing and I want "thread safe" engineering.

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

(aside)
Hehe. FWIW, I deal with "free" issues (and the pesky GCC const warnings)
using a static inline _free() routine. I can will expose my _free() (its already
used pretty heavily internally to POPT) as poptFree() if you wish. So far I've 
just
tried not to inflict my C coding fetishism on anyone else. It is *in fact*
just laziness on the part of the developer to test for NULL and add the
silly cast. Developer laziness is not a POPT problem per se.

> 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");
> 

My guess (not looked, just from memory) is that poptPeekArg _SHOULD_ return
malloc'd arguments.

The underlying issue is that POPT has a ar filtering construct based on the
gawd awful bash syntax that looks like (here's an example from 
/usr/lib/rpm/rpmpopt-X.Y)

#   [--dbpath DIRECTORY""use database in DIRECTORY"
rpm alias --dbpath  --define '_dbpath !#:+'

The --dbpath option is quite important for RPM and the ^%$^#^$^%#^%#$^% "!#:+"
syntax was quite useful in ripping out _LOTS_ of tedious code in RPM setting
what is essentially just a file path string argument to an option.

The relation to "memory leaks" is that POPT had to start actively parsing
argv[0] arrays in order to pull the next argument, and that turned out
to be easier to do if arguments were malloc'd before return to caller.

So poptPeekArg() _SHOULD_ return malloc'd memory (but may not do so atm
because I know that nothing but RPM has ever really used poptPeekArg() and
I've been fighting to simplify RPM arg handling for the last 10 years, mostly
successfully).


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

poptGetOptArg() is definitely malloc'd, that is the valgrind trace I posted.

I deal with the issue like this (again from RPM code in lib/poptALL.c with
spling pugliness left in cut-n-paste):

/* Process all options, whine if unknown. */
  

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

[Danny Sung]

On 6/7/2010 11:29 AM, Jeff Johnson wrote:


On Jun 7, 2010, at 2:00 PM, Danny Sung wrote:


I agree.  I have no problems with breaking binary&source compatibility especially if 
that change will make popt better/easier in the future. But a change in name seems 
necessary for app developers to make that choice.  eg. what happens if you're building 
mulitiple different software packages... both dynamically linking popt, but one uses 1.x 
and the other 2.x?  The header files would also need something like 
#include  or something like that.  Can't say I particularly like 
it... but it does those issues somewhat cleanly.



I respectfully and strongly _DISAGREE_. The issue can be best be
dealt with by using ELF symbol versioning (once I get that it place
again in popt-1.17) where a single library can provide _BOTH_
POPT 1.0 and POPT 2.0 functionality _TRANSPARENTLY_ (one the
necessary pre-requsite of adding ELF symbol versioning is achieved).


Doesn't this only work if you're maintaining source compatibility? 
(whiihc is good, I just thought you said you weren't aiming for that).
Sorry, not trying to be difficult... just trying to understand.  Also 
does ELF symbol versioning work for both dynamically and statically 
linked apps?



If I rename "popt" to "tdod" (or "popt2" that precludes even the attempt at
doing better engineering using ELF symbol versioning. While I'm
not averse to renaming, I've seen the bleary package churn from
libxml ->  libxml2 and GNOME 1.0 ->  GNOME 2.0. No way Jose!


Yeah, I thought it was ugly too.  But it makes sense... in an ugly way.
__
POPT Library   http://rpm5.org
Developer Communication List   popt-devel@rpm5.org

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

[Danny Sung]

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.

__
POPT Library   http://rpm5.org
Developer Communication List   popt-devel@rpm5.org

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

[Jeff Johnson]

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.

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

Yes. I've added const's wherever possible and necessary in popt.
Which is why I already know (from 10+ years of experience) that only
PROT_READ segfaults will communicate to developers that its _NOT_
a POPT bug.

> 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?

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

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.

OTOH, some of the code paths in POPT are quite twisty, perhaps there's someplace
where another strdup/malloc is needed.

73 de Jeff
__
POPT Library   http://rpm5.org
Developer Communication List   popt-devel@rpm5.org

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

[Jeff Johnson]

On Jun 7, 2010, at 2:00 PM, Danny Sung wrote:

> I agree.  I have no problems with breaking binary&source compatibility 
> especially if that change will make popt better/easier in the future. But a 
> change in name seems necessary for app developers to make that choice.  eg. 
> what happens if you're building mulitiple different software packages... both 
> dynamically linking popt, but one uses 1.x and the other 2.x?  The header 
> files would also need something like #include  or something 
> like that.  Can't say I particularly like it... but it does those issues 
> somewhat cleanly.
> 

I respectfully and strongly _DISAGREE_. The issue can be best be
dealt with by using ELF symbol versioning (once I get that it place
again in popt-1.17) where a single library can provide _BOTH_
POPT 1.0 and POPT 2.0 functionality _TRANSPARENTLY_ (one the
necessary pre-requsite of adding ELF symbol versioning is achieved).

If I rename "popt" to "tdod" (or "popt2" that precludes even the attempt at
doing better engineering using ELF symbol versioning. While I'm
not averse to renaming, I've seen the bleary package churn from
libxml -> libxml2 and GNOME 1.0 -> GNOME 2.0. No way Jose!

For libraries and projects of XML/GNOME size and magnitude, renaming
may have been the only viable way forward. OTOH, popt is _TINY_
and I'm quite prepared for ELF symbol versioning (and I also know
that I can figger out whether a pointer is to a POPT 1.0 or a
Yet-To-Be-Devised POPT 2.0 table without too much pain. Won't
be pretty code, and I am a lazy schmuck maintainer, but I _KNOW_ that
I can solve the problem).

The only other reason for renaming is to signal developers _EVERYWHERE_
that a change just happened. But that assumes that I am willing to describe
the changes ad nauseam for the next couple of years, which is very
much not the case.

I much prefer "drop-in" transparent "compatibility" and am unwilling to
preclude that engineering path by simply wussing out and
renaming "popt" to "tdod" (or "popt2" or "getopt106" or ...) which
doesn't begin to solve _ANY_ "compatibility" issue at all. In fact,
renaming just sanitizes "incompatibility" without solving any problem.

> I don't think it's that bad for developers to switch over.  Old apps may take 
> a while.  But people will know (especially if you put "#warning popt-1.x is 
> deprecated" in the headers for 1.x. =) ).  And will move new software over.  
> Look at things like libxml2.
> 
> Ideally, however, popt2 should include mechanisms that allow for future 
> versions to do this sort of versioning check at runtime.  So this popt2 
> actually becomes popt ABI 2.0.  Not popt-2.0.  (eg. popt3 may still use popt2 
> ABI.. but then again with all the versioning there may never need to be a 
> popt3 =) ).
> 

The project name is "POPT", the version being discussed is "2.0".

There is no "popt2" nor (imho) is there any need for the renaming,
nor setting the precedent of "popt3" -> ... -> "popt123456789" in the 30th 
century.

73 de Jeff

> 
> On 6/7/2010 9:51 AM, Wayne Davison wrote:
>> On Mon, Jun 7, 2010 at 9:40 AM, Jeff Johnson > > wrote:
>> 
>>The added tyrrany of forcing every application that currently has
>>-lpopt
>>to change to
>>-ljdod
>>will be rate-determining to adoption (and glacially/tectonically
>>slow imho)
>> 
>> 
>> For me, if popt 2 is not compatible with popt 1 then I would rather have
>> to make a conscious choice to upgrade my code (testing it for
>> compatibility), and having to change the libray name as a part of that
>> is pretty minor.  Having a possibility for my program to be run-time
>> linked with a library that is not compatible with what it expects would
>> be very bad, imo.
>> 
>> ..wayne..
> 
> -- 
> Please note my new email address: da...@dannysung.com
> __
> POPT Library   http://rpm5.org
> Developer Communication List   popt-devel@rpm5.org

__
POPT Library   http://rpm5.org
Developer Communication List   popt-devel@rpm5.org

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

[Danny Sung]

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 

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

[Danny Sung]

I agree.  I have no problems with breaking binary&source compatibility 
especially if that change will make popt better/easier in the future. 
But a change in name seems necessary for app developers to make that 
choice.  eg. what happens if you're building mulitiple different 
software packages... both dynamically linking popt, but one uses 1.x and 
the other 2.x?  The header files would also need something like #include 
 or something like that.  Can't say I particularly like 
it... but it does those issues somewhat cleanly.


I don't think it's that bad for developers to switch over.  Old apps may 
take a while.  But people will know (especially if you put "#warning 
popt-1.x is deprecated" in the headers for 1.x. =) ).  And will move new 
software over.  Look at things like libxml2.


Ideally, however, popt2 should include mechanisms that allow for future 
versions to do this sort of versioning check at runtime.  So this popt2 
actually becomes popt ABI 2.0.  Not popt-2.0.  (eg. popt3 may still use 
popt2 ABI.. but then again with all the versioning there may never need 
to be a popt3 =) ).



On 6/7/2010 9:51 AM, Wayne Davison wrote:

On Mon, Jun 7, 2010 at 9:40 AM, Jeff Johnson mailto:n3...@mac.com>> wrote:

The added tyrrany of forcing every application that currently has
-lpopt
to change to
-ljdod
will be rate-determining to adoption (and glacially/tectonically
slow imho)


For me, if popt 2 is not compatible with popt 1 then I would rather have
to make a conscious choice to upgrade my code (testing it for
compatibility), and having to change the libray name as a part of that
is pretty minor.  Having a possibility for my program to be run-time
linked with a library that is not compatible with what it expects would
be very bad, imo.

..wayne..


--
Please note my new email address: da...@dannysung.com
__
POPT Library   http://rpm5.org
Developer Communication List   popt-devel@rpm5.org

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

[Jeff Johnson]

On Jun 7, 2010, at 12:51 PM, Wayne Davison wrote:

> On Mon, Jun 7, 2010 at 9:40 AM, Jeff Johnson  wrote:
> The added tyrrany of forcing every application that currently has
>   -lpopt
> to change to
>   -ljdod
> will be rate-determining to adoption (and glacially/tectonically slow imho)
> 
> For me, if popt 2 is not compatible with popt 1 then I would rather have to 
> make a conscious choice to upgrade my code (testing it for compatibility), 
> and having to change the libray name as a part of that is pretty minor.  
> Having a possibility for my program to be run-time linked with a library that 
> is not compatible with what it expects would be very bad, imo.
> 

Last I checked you already had made your choice and included your own
private version of -lpopt (and the reasons are perfectly understood and have
nothing to do with POPT 2.0).

Your decision paths are all status quo ante no matter what is done (or not) in 
POPT 2.0.

And I point out that your decision paths are all independent of whether POPT 
2.0 is
"compatible" or "renamed" or ... anything else.

Yes, your comments are valued, you send patches ;-)


Meanwhile the discussion on  is intended to
explictly extract what problems/features need to be solved in POPT 2.0
before doing any coding.

(aside)
I almost added a RPN calculator to popt-1.16 this weekend past.
The implementation would have involved a table entry like

int counter = 0;

  { "autoincrement", '\0', POPT_ARG_INT|POPT_ARGFLAG_RPN,   &counter, 0,
N_("Demonstrate autoincrmenting using a RPN calculator"), "1 +"  },

where a simple uint64_t stack would push the existing value (with coercion), 
parse simple
digit strings onto the uint64_t stack, with the usual toy calculator arithmetic
operations of + - * / % & | ... , and then transferring the result (again with 
coercion)
back to the counter variable.

The exercise of in-fix -> reverse polish is left to inquiring minds. Hint: see 
wikipedia.

Alas I got (and am) side-tracked with RSA/DSA/Elgamal/ECDSA generate & sign 
methods
for RPM, so I did not get around to the RPN calculator and your "autoincrement" 
request.


73 de Jeff

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

[Wayne Davison]

On Mon, Jun 7, 2010 at 9:40 AM, Jeff Johnson  wrote:

> The added tyrrany of forcing every application that currently has
> -lpopt
> to change to
> -ljdod
> will be rate-determining to adoption (and glacially/tectonically slow imho)
>

For me, if popt 2 is not compatible with popt 1 then I would rather have to
make a conscious choice to upgrade my code (testing it for compatibility),
and having to change the libray name as a part of that is pretty minor.
 Having a possibility for my program to be run-time linked with a library
that is not compatible with what it expects would be very bad, imo.

..wayne..

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

[Jeff Johnson]

On Jun 7, 2010, at 12:20 PM, Wayne Davison wrote:

> On Mon, Jun 7, 2010 at 9:14 AM, Jeff Johnson  wrote:
> And without some deterministic way to tell whether its a POPT 1.x <-> 2.x
> table being fed to the POPT API/ABI, well, only a deliberately
> "incompatible" POPT 2.0 data structure with some deterministic
> version identifier is possible.
> 
> One way to deal with this is to change the library name to "popt2".  That 
> way, a popt (1.x) using program would never get run with a popt2 library.
> 

OK, let's go down that path ...

Assume I  change the name of "popt" to "jdod" (ASCII art upsidedown and 
reversed to make a point)

What "compatibility" problem is solved by renaming?

Doesn't  fiddling the loader map to change the symbol versioning
achieve "incompatibility" a bit more transparently.

(aside)
Eeek, no symbol versioning in existing -lpopt, time to fix that with popt-1.17, 
todo++.

The added tyrrany of forcing every application that currently has
-lpopt
to change to
-ljdod
will be rate-determining to adoption (and glacially/tectonically slow imho)

So slow that it may not even be worth the effort of developing and releasing 
POPT 2.0.
That too is an option.

(aside)
There's no easy answer and I'm deeply conflicted, but am being encouraged
to attempt a POPT 2.0 release to "fix" certain issues like --help. Go Google 
Rusty Russell's
POPT code review. He (and I) want to see callbacks used, and a callback
paradigm Done Right forces a "void *" context pointer like this (AFAICT)

int rc = (*callback) (, void * callback_arg)

which means that I either
1) overload the i18n field pointer(s) with Yet Another Obscure 
Overloading (the
per-table pre- and soit- and post- callbacks are already quite tricky 
to use correctly)
2) add another pointer field to avoid overloading and introduce 
"instant incompatibility"

I can go either/both/neither way. My interest is in useful and simple software, 
nothing more.

Again, I use "jdod" just to illustrate why renaming to -lpopt2 is just the tip 
of a large iceberg.
(you know that already).

73 de Jeff

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

[Wayne Davison]

On Mon, Jun 7, 2010 at 9:14 AM, Jeff Johnson  wrote:

> And without some deterministic way to tell whether its a POPT 1.x <-> 2.x
> table being fed to the POPT API/ABI, well, only a deliberately
> "incompatible" POPT 2.0 data structure with some deterministic
> version identifier is possible.
>

One way to deal with this is to change the library name to "popt2".  That
way, a popt (1.x) using program would never get run with a popt2 library.

..wayne..

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

[Jeff Johnson]

On Jun 7, 2010, at 12:02 PM, Jeff Johnson wrote:
> 
> Changing the soname or even (gasp!) using ELF symbol versioning
> is quite doable, all the necessary precursor elements have been in place
> for years.
> 

I should point out the deep flaw using ELF symbol versioning ...

POPT tables are compiled into applications, and so versioning is
dictated by the application, not by POPT itself.

And without some deterministic way to tell whether its a POPT 1.x <-> 2.x
table being fed to the POPT API/ABI, well, only a deliberately
"incompatible" POPT 2.0 data structure with some deterministic
version identifier is possible.

But perhaps there's some insanely obscure way to disambigauate
POPT tables that I haven't been able to devise. Have at and
send a patch, I really loathe being out guessed (because I'm out
numbered by zillions of POPT lusers to essentially 1 developer)
since forever.

I have thought long and hard for many years ... "incompatible" POPT 2.0 seems
the easiest (though rocky) path into the future.

73 de Jeff
__
POPT Library   http://rpm5.org
Developer Communication List   popt-devel@rpm5.org

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

[Jeff Johnson]

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

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

[Mark Hatle]

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.

For runtime, it should be a simple 'switch' for the strdup call.


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



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.


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


--Mark


0) overloading with bit masks and long <-> void * punning is quite 
mysterious to most.

1) bits in certain fields are _ALREADY_ ambiguous for certain values, 
with
subtle "conventional" contextual tests of other entries needed while 
using POPT.

2) there's a need for Yet Another Pointer for callbacks with context 
(changing item #5
from "int" to "long" would suffice but is "instantly incompatible" so 
I'd rather do something
less mysterious).

Thanks for comments however. And ignore my scarcasm if it offends.

73 de Jeff
__
POPT Library   http://rpm5.org
Developer Communication List   popt-devel@rpm5.org


__
POPT Library   http://rpm5.org
Developer Communication List   popt-devel@rpm5.org

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

[Jeff Johnson]

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.

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

> 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:

0) overloading with bit masks and long <-> void * punning is quite 
mysterious to most.

1) bits in certain fields are _ALREADY_ ambiguous for certain values, 
with
subtle "conventional" contextual tests of other entries needed while 
using POPT.

2) there's a need for Yet Another Pointer for callbacks with context 
(changing item #5
from "int" to "long" would suffice but is "instantly incompatible" so 
I'd rather do something
less mysterious).

Thanks for comments however. And ignore my scarcasm if it offends.

73 de Jeff
__
POPT Library   http://rpm5.org
Developer Communication List   popt-devel@rpm5.org

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

[Mark Hatle]

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.


Either by adding const style prototypes/functions, or by using some mechanism to 
change the behavior of all of the functions.


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.


--Mark

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


__
POPT Library   http://rpm5.org
Developer Communication List   popt-devel@rpm5.org

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

[Jeff Johnson]

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