Re: [PATCH] rerere forget: grok files containing NUL

From: Johannes Sixt <j6t@kdbg.org>
Date: Apr 2, 2013, 19:03 UTC
Message-ID: <515B2B6D.4050801@kdbg.org>
In-Reply-To: <7vhajpj294.fsf@alter.siamese.dyndns.org>

Am 02.04.2013 00:48, schrieb Junio C Hamano:

Show 11 quoted lines

> Johannes Sixt <j6t@kdbg.org> writes:
> 
>> Using 'git rerere forget .' after a merge that involved binary files
>> runs into an infinite loop if the binary file contains a zero byte.
>> Replace a strchrnul by memchr because the former does not make progress
>> as soon as the NUL is encountered.
> 
> Hmph, thanks.
> 
> Is it the right behaviour for rerere to even attempt to interfere
> with a merge that involves binary files in the first place?

Why not? And how could rerere tell the difference? It would have to do the same check for binary-ness as the merge driver before it even starts looking closer at the files.

> Does the three-way merge machinery replay recorded resolution for
> such a binary file correctly (after your fix, that is)?

Yes, it does. It recognizes the binary-ness and picks 'our' side. Only then comes rerere_mem_getline into play.

Show 17 quoted lines

>> diff --git a/rerere.c b/rerere.c
>> index a6a5cd5..4d940cd 100644
>> --- a/rerere.c
>> +++ b/rerere.c
>> @@ -284,8 +284,10 @@ static int rerere_mem_getline(struct strbuf *sb, struct rerere_io *io_)
>>  	strbuf_release(sb);
>>  	if (!io->input.len)
>>  		return -1;
>> -	ep = strchrnul(io->input.buf, '\n');
>> -	if (*ep == '\n')
>> +	ep = memchr(io->input.buf, '\n', io->input.len);
>> +	if (!ep)
>> +		ep = io->input.buf + io->input.len;
>> +	else if (*ep == '\n')
>>  		ep++;
>>  	len = ep - io->input.buf;
>>  	strbuf_add(sb, io->input.buf, len);