Re: [PATCH v4 01/11] argv_array: offer to split a string by whitespace

[Junio C Hamano <gitster@pobox.com>]

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> +void argv_array_split(struct argv_array *array, const char *to_split)
> +{
> +	while (isspace(*to_split))
> +		to_split++;
> +	for (;;) {
> +		const char *p = to_split;
> +
> +		if (!*p)
> +			break;
> +
> +		while (*p && !isspace(*p))
> +			p++;
> +		argv_array_push_nodup(array, xstrndup(to_split, p - to_split));

Can *p be '\0' at this point?  If isspace('\0') behaves sensibly,
then we wouldn't have had to check "*p &&" in the previous while()
loop.  If not, then while() on the next line needs the same "*p &&"
check.  I think ou r isspace('\0') is reliably false, so we could
drop "*p &&" but I do not mind spelling it out that we care about
end of string explicitly.  I however think we would want to be
consistent inside a single loop which stance we take.

> +		while (isspace(*p))
> +			p++;
> +		to_split = p;
> +	}
> +}

I wonder if the initial "skip spaces" can come inside the main loop,
perhaps like so:

	for (;;) {
		const char *p = to_split;

		while (*p && isspace(*p++))
			;
		if (!*p)
			break;
		for (to_split = p; *p && !isspace(*p); p++)
			;
		argv_array_push_nodup(array, xstrdup(to_split, p - to_split));
	}

or something.

Duplicated "skip over spaces" loop is not a big deal, though.