Re: [PATCH-v2/RFC 2/6] xutils: fix hash with whitespace on incomplete line

Thell Fowler <git@tbfowler.name> writes:

Please study the style of existing commit messages and imitate them.

Thanks.

The issue you identified and tried to fix is a worthy one. But before the pre-context of this hunk, I notice these lines:

		if (isspace(*ptr)) {
			const char *ptr2 = ptr;
			while (ptr + 1 < top && isspace(ptr[1])
					&& ptr[1] != '\n')
				ptr++;

If you have trailing whitespaces on an incomplete line, ptr initially points at the first such whitespace, ptr2 points at the same location, and then the while() loop advances ptr to point at the last byte on the line, which in turn will be the last byte of the file. And the codepath with your updates still try to access ptr[1] that is beyond that last byte.

I would write it like this patch instead.

The intent is the same as your patch, but it avoids accessing ptr[1] when that is beyond the end of the buffer, and the logic is easier to follow as well.

-- >8 --
Subject: xutils: fix hashing an incomplete line with whitespaces at the end

Upon seeing a whitespace, xdl_hash_record_with_whitespace() first skipped the run of whitespaces (excluding LF) that begins there, ensuring that the pointer points the last whitespace character in the run, and assumed that the next character must be LF at the end of the line. This does not work when hashing an incomplete line, that lacks the LF at the end.

Introduce "at_eol" variable that is true when either we are at the end of line (looking at LF) or at the end of an incomplete line, and use that instead throughout the code.

Noticed by Thell Fowler.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 xdiff/xutils.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)