Linux has a really good library called popt. It parses command line arguments.
I use it all the time, and it allows a variety of arguments to be cleanly handled, with different types, default values, help text, and so on.
For a long time it has bugged me, and I assumed it was a bug, that it would leak a small amount of memory. This shows using valgrind. But as it is a one-off leak on a command line I have not bothered too much.
However, following some discussions re memory free tidying on one command recently I thought I would try and get my code 100% leak free - always a good aim.
I have found the problem.
popt malloc's any POPT_ARG_STRING values that it sets!
Now, I was not surprised to find some malloc'd memory, but was surprised at this. I was also surprised that poptFreeContext() does not free all allocated memory, these values are for the user to free.
What concerned me is that I did not know this, and the manual pages are not entirely clear. They do actually explain that poptGetOptArg() returns final arg of poptGetNextOpt(), and "The calling function is responsible for deallocating this string.". It does not seem to make clear that all arguments returned or stored for POPT_ARG_STRING are malloc'd. But that seems to be the case.
Now I know, I can ensure I free all string variables at the end of my program. Or can I?
What of defaults?
Well, damn, this is fun - some variables have defaults (the help text shows them even, if you ask it too), so you can set a default and the use popt, and have a string that is supplier, or the default.
This is useful, until you start freeing all your strings at the end, as these are typically const char assigned, and so not something you can free.
The only way to be clean is assign your defaults using strdup(), so you can always free, whether default or stored by popt.
Why does it do it?
What puzzles me is why it is done in the first place. The arguments are null terminated strings in argv[], so could be referenced directly. Even with a -x or --xname= prefix, an offset in to the argv[] value could be returned.
It is worse!
It seems poptGetArg() also alloc's strings as well, and the manual is totally silent on that point.
Arg! And worse, poptGetArg() is const char*, which is not valid for free() even!
No, No, that does not work
OK, I really tried, honest.
I set my defaults to be strdup() based so they are always safe to free() whether using default or supplied value.
What does that do? Well, if not using the default, the new supplied value is set, and malloc'd, so the free() is still safe. But what of the original default, that is now lost, not free'd, so a bloody leak!
I cannot see any clean way to do this - popt is basically broken.
Arrrg!
I never had any trouble just parsing argv directly.
ReplyDeleteThis is why I write all non-trivial command line tools in Rust.
ReplyDeleteI can't help but notice an inconsistency between the first and (second to) last sentences in your post...
ReplyDeleteCan you coerce popt into giving you a pointer to the default? Then you could stash it away and free it yourself -- but honestly by this point it's probably easier to fix it in popt. Only, that's hard, because countless applications already depend on its existing rather, er, woolly semantics, and you can hardly go changing its ABI at whim to fix a leak of a few bytes.
ReplyDeleteWrite your own allocator with an is_in_heap(p) function, then you can do
ReplyDeleteif (is_in_heap(p)) free(p);