Re: [PATCH 1/3] refactor dir_add_name

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 11, 2007, 16:23 UTC
Message-ID: <7vk5uaqx3q.fsf@assigned-by-dhcp.pobox.com>
In-Reply-To: <20070611133944.GA7008@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> If we like the alloc_grow approach, there are a lot of places where we
> can drop a 3-5 line conditional into a single line. I find it much more
> readable, but others may disagree.

I like the readability, but

Show 18 quoted lines

>  cache.h |    6 ++++++
>  dir.c   |   23 +++++++++++------------
>  2 files changed, 17 insertions(+), 12 deletions(-)
>
> diff --git a/cache.h b/cache.h
> index 5e7381e..f771519 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -224,6 +224,12 @@ extern void verify_filename(const char *prefix, const char *name);
>  extern void verify_non_filename(const char *prefix, const char *name);
>  
>  #define alloc_nr(x) (((x)+16)*3/2)
> +#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 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 is. A comment before the macro, and a space between 'if' and opening parenthesis, would be good things to have.