From: Jeff Hostetler <git@jeffhostetler.com>
Date: Thu, 09 Mar 2017 15:46:53 GMT
Subject: Re: [PATCH 01/10] pack-objects: eat CR in addition to LF after fgets.
Message-ID: <52221b43-0c38-97a2-d96d-0dedf5f45796@jeffhostetler.com>
In-Reply-To: <20170309070128.we6hbraea5am3ado@sigill.intra.peff.net>



On 3/9/2017 2:01 AM, Jeff King wrote:
> On Wed, Mar 08, 2017 at 05:37:56PM +0000, Jeff Hostetler wrote:
>
>> From: Jeff Hostetler <git@jeffhostetler.com>
>>
>> Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
>> ---
>>  builtin/pack-objects.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
>> index f294dcf..7e052bb 100644
>> --- a/builtin/pack-objects.c
>> +++ b/builtin/pack-objects.c
>> @@ -2764,6 +2764,8 @@ static void get_object_list(int ac, const char **av)
>>  		int len = strlen(line);
>>  		if (len && line[len - 1] == '\n')
>>  			line[--len] = 0;
>> +		if (len && line[len - 1] == '\r')
>> +			line[--len] = 0;
>
> Rather than add features to this bespoke line-reader, can we switch this
> to use strbuf_getline()? That handles line endings, and avoids the
> awkward corner case where fgets "breaks" a long line across two calls.
>
> Something like the patch below. I suspect read_object_list_from_stdin()
> should get the same treatment.

Much nicer.  Will do.  Thanks!

>
> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> index 76b1919ca..6b9fffe9c 100644
> --- a/builtin/pack-objects.c
> +++ b/builtin/pack-objects.c
> @@ -2765,7 +2765,7 @@ static void record_recent_commit(struct commit *commit, void *data)
>  static void get_object_list(int ac, const char **av)
>  {
>  	struct rev_info revs;
> -	char line[1000];
> +	struct strbuf buf = STRBUF_INIT;
>  	int flags = 0;
>
>  	init_revisions(&revs, NULL);
> @@ -2775,12 +2775,12 @@ static void get_object_list(int ac, const char **av)
>  	/* make sure shallows are read */
>  	is_repository_shallow();
>
> -	while (fgets(line, sizeof(line), stdin) != NULL) {
> -		int len = strlen(line);
> -		if (len && line[len - 1] == '\n')
> -			line[--len] = 0;
> -		if (!len)
> +	while (strbuf_getline(&buf, stdin) != EOF) {
> +		const char *line = buf.buf;
> +
> +		if (!buf.len)
>  			break;
> +
>  		if (*line == '-') {
>  			if (!strcmp(line, "--not")) {
>  				flags ^= UNINTERESTING;
> @@ -2800,6 +2800,7 @@ static void get_object_list(int ac, const char **av)
>  		if (handle_revision_arg(line, &revs, flags, REVARG_CANNOT_BE_FILENAME))
>  			die("bad revision '%s'", line);
>  	}
> +	strbuf_release(&buf);
>
>  	if (use_bitmap_index && !get_object_list_from_bitmap(&revs))
>  		return;
>