[PATCH 2 of 2] osutil.c: replace alloca() with malloc() and remove <alloca.h>

Giorgos Keramidas keramida at ceid.upatras.gr
Tue Oct 9 06:16:54 CDT 2007


On 2007-10-08 10:18, Matt Mackall <mpm at selenic.com> wrote:
> On Sun, Oct 07, 2007 at 10:00:31PM +0300, Giorgos Keramidas wrote:
> > # HG changeset patch
> > # User Giorgos Keramidas <keramida at ceid.upatras.gr>
> > # Date 1191783522 -10800
> > # Node ID 5d13d2d3f42bc150f711bfa3a8bb38a7d0b5c1d4
> > # Parent  37dc732e00416a50dcc9b994961acdb6ceb755ba
> > osutil.c: replace alloca() with malloc() and remove <alloca.h>
> >
> > The <alloca.h> header is not really portable, and it breaks Mercurial on
> > FreeBSD.  Remove the dependency on <alloca.h> and alloca() by replacing
> > the only place where it was used with malloc().
>
> It seems the most sensible thing to do here is get rid of the call
> entirely and do:
>
> 	char full_path[PATH_MAX + epsilon];
>
> up in the local variable declarations. Combined with strncat instead
> of the current strcpy, of course.

This inserts an arbitrary `epsilon' based limit to `full_path[]', and
may not work well in all cases.  The current calculation of `full_path's
size depends on `path_len', which is set by:

    {
        static char *kwlist[] = { "path", "stat", NULL };
        PyObject *statobj = NULL;

        if (!PyArg_ParseTupleAndKeywords(args, kwargs, "s#|O:listdir", kwlist,
                                         &path, &path_len, &statobj))
            goto bail;

        do_stat = statobj && PyObject_IsTrue(statobj);
    }

Then `full_path' is allocated with one of:

    full_path = alloca(path_len + PATH_MAX + 2);

    full_path = malloc(path_len + PATH_MAX + 2);

Either one can allocate as much memory as needed, so I'd prefer to keep
one of the dynamic allocation functions instead of a static array size.

Of the dynamic allocation functions I prefer malloc(), because alloca()
is slightly unsafe (there is no "clean" way to portably verify that
alloca() exists, that it works correctly, that it doesn't overflow or
underflow the stack), and so far I haven't been able to measure any
speed penalty when malloc() is used.



More information about the Mercurial-devel mailing list