Subject: Re: [PATCH 1/2] rev list add option accepting revision constraints on standard input

From: Junio C Hamano <[email-masked]>
Date: Tue, 05 Sep 2006 15:10:54 -0700

Andy Whitcroft <[email-masked]> writes:

> Add a --stdin flag which causes rev-list to additionally read
> its stdin stream and parse that for revision constraints.

> +/*
> + * Parse revision information, filling in the "rev_info" structure,
> + * revisions are taken from stream.
> + */
> +static void setup_revisions_stream(FILE *stream, struct rev_info *revs)
> +{
> +	char line[1000];
> +	const char *args[] = { 0, line, 0 };
> +
> +	while (fgets(line, sizeof(line), stream) != NULL) {
> +		line[strlen(line) - 1] = 0;
> +
> +		if (line[0] == '-')
> +			die("options not supported in --stdin mode");
> +
> +		(void)setup_revisions(2, args, revs, NULL);
> +	}
> +}

Is calling setup_revisions() on the same revs like this many
times safe?  I do not think so, especially what is after the
primary "for()" loop in the function.

I was sort-of expecting that you would instead replace that
primary for() loop in setup_revisions() with some sort of
callback...