From: Junio C Hamano <junkio@cox.net>
Date: Tue, 05 Sep 2006 22:10:54 GMT
Subject: Re: [PATCH 1/2] rev list add option accepting revision constraints on standard input
Message-ID: <7vpseaarrl.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <20060905215157.GA29172@shadowen.org>

Andy Whitcroft <apw@shadowen.org> 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...