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

The reason for "discouraged" is, of course, the stack is smashed when  
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  

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

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  
> 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  
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  
the necessary i18n marker:

poptHelpOptions, \
                         0, N_("Help options:"), NULL },

Bumping the soname and removing the hack needs to be done somewhen.

> Any objections?


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.