RPM Community Forums

Mailing List Message of <popt-devel>

Re: [PATCH] get rid of ancient alloca(3) use

From: Jeff Johnson <n3npq@mac.com>
Date: Thu 14 Jun 2007 - 14:45:38 CEST
Message-Id: <0F67DD01-A521-43FF-B4C4-3EA911874FF6@mac.com>

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
Received on Thu Jun 14 14:47:48 2007
Driven by Jeff Johnson and the RPM project team.
Hosted by OpenPKG and Ralf S. Engelschall.
Powered by FreeBSD and OpenPKG.