Re: [PATCH] gc: fix off-by-one in append_option

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 18, 2012, 19:34 UTC
Message-ID: <7vd374ltqh.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20120418191849.GA12619@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 19 quoted lines

> I've included a patch below that makes this look like:
>
>   static const char repack_cmd[] = {"repack", "-d", "-l", NULL };
>   static struct argv_array repack = ARGV_ARRAY_INIT_DEFAULT(repack_cmd);
>
> which is way less nice than I would prefer, but I think may be the best
> we can do with static initialization.
>
>> What do you think of just:
>> 
>>   static struct argv_array argv_repack;
>>   [...]
>>   argv_init_from_string(&argv_repack, "repack -d -l");
>
> I looked into this, but it seems our sq_dequote_* functions insist that
> it be spelled "'repack' '-d' '-l'", so I would have to teach them to be
> more liberal first.
>
> Thoughts?

I do not know it is worth it to try to be too fancy.

I was about to suggest, immediately after seeing the first one I quoted above, to omit NULL and instead use ARRAY_SIZE(), but I do not think that is even worth it, as some (possibly future) caller may have only "char **" as a usual NULL terminated array at hand.

I am perfectly OK with even without initializers, like this:

	struct argv_array repack = ARGV_ARRAY_INIT;
	argv_array_push_strings(&repack, "repack", "-d", "-l", NULL);