On Jun 14, 2007, at 8:11 AM, Ralf S. Engelschall wrote:
I have one subtle issue on my RPM/POPT todo list since a longer
time: POPT's use of the ancient 32V AT&T UNIX alloca(3), a machine-,
compiler-, and system-dependent and especially non-POSIX (and hence
less portable) function whose use is even discouraged on lots of Unix
platforms.
There are those who like alloca and those who don't. ;-)
Please cite your personal preferences/concerns rather than the arch
"ancient" and "discouraged".
(aside)
The reason for "discouraged" is, of course, the stack is smashed when
alloca
is misused or abused, so a backtrace may not exist or a hard to see
exploit may
be introduced through an overrrun. These are usually C101 coding
concerns.
POPT even uses it in a IMHO unecessary way to allocate temporary
structures on the stack:
poptDone done = alloca(sizeof(*done));
This can be just replaced by the following and let the compiler do
mostly all the work already under compile-time:
struct poptDone_s done_buf;
poptDone done = &done_buf;
This is likely my changes, often driven by splint nagging.
My criteria for the change were likely one line of code instead of
two, and hiding
"struct poptDone_s" everywhere possible. You may find similar style
throughout rpm.
(aside)
I like
typedef struct poptDone_s * poptDone;
rather than
typedef struct poptDone_s poptDone
The following patch completely kicks out all alloca(3) usages from
POPT.
Either through the above replacement, or by not requiring a temporary
buffer at all (by passing length argument to internal findOption()
function) or in the worst cases by replacing with the POSIX malloc(3)
and calloc(3) functions.
BTW, I also use xmalloc/xcalloc/xstrdup which are also tricked up so
that
mtrace/valgrind report useful memory leak info in the backtrace. See
top level system.h.
Bringing xmalloc et al into popt might be useful, its silly to waste
a lot
of coding effort checking for NULL returns from malloc. I have yet to
ever encounter error recovery that might be attempted when
malloc returns NULL. An assert immediate exit is what I prefer. YMMV.
I carefully performed the replacements, reviewed them individually and
the POPT "make check" still works, of course.
As long as "make check" passes, all changes to popt are fair game.
Note the dreadful hack in popthelp.c too. This popt.h define did not
include
the necessary i18n marker:
#define POPT_AUTOHELP { NULL, '\0', POPT_ARG_INCLUDE_TABLE,
poptHelpOptions, \
0, N_("Help options:"), NULL },
Bumping the soname and removing the hack needs to be done somewhen.
Any objections?
Nope.
73 de Jeff
______________________________________________________________________
POPT Library http://rpm5.org
Developer Communication List [email protected]
