On Mon, Jun 11, 2007 at 09:23:05AM -0700, Junio C Hamano wrote:
> > +#define alloc_grow(x, nr, alloc) do { \
> > + if(nr >= alloc) { \
> > + alloc = alloc_nr(alloc); \
> > + x = xrealloc((x), alloc * sizeof(*(x))); \
> > + } \
> > +} while(0)
>
> worry a bit about macro safety. I think the presence of an
Agreed. The multiple evaluation of 'x' means that growing foo[i++] will
cause quite a confusing bug. I would prefer it as an inline, but at
least the sizeof requires macro magic. Not to mention the ugliness of
moving all those lvalues as pointers. But we could do something like
(totally untested):
#define alloc_grow(x, nr, alloc) \
alloc_grow_helper(&(x), nr, &(alloc), sizeof(*(x)))
inline
void alloc_grow_helper(void **x, int nr, int *alloc, int size)
{
if (nr >= *alloc) {
*alloc = alloc_nr(*alloc);
*x = xrealloc(*x, *alloc * size);
}
}
Horribly ugly (I'm seeing stars!) but probably a bit safer in the long
run, and nobody needs to look at it most of the time. :)
What do you think?
> assignment to alloc and x would make sure we would catch an
> error to pass non lvalue as 'alloc' and 'x', so it may be Ok as
Even lvalues can have side effects, so multiple evaluation is still a
problem.
> is. A comment before the macro, and a space between 'if' and
> opening parenthesis, would be good things to have.
Yes, sorry, my fingers are always forgetting the git whitespace
conventions. I (or Jonas) will submit a better commented version, but do
let us know which version you like.
-Peff
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
