Junio C Hamano (gitster@xxxxxxxxx) wrote on Aug 23, 2009:
> Thell Fowler <git@xxxxxxxxxxxxx> writes:
>
> > - Make xdl_hash_record_with_whitespace stop hashing before the
> > eof when ignoring space change or space at eol on an incomplete
> > line.
> >
> > Resolves issue with a final trailing space being included in the
> > hash on an incomplete line by treating the eof in the same fashion
> > as a newline.
>
> Please study the style of existing commit messages and imitate them.
>
I'll keep trying.
> > Signed-off-by: Thell Fowler <git@xxxxxxxxxxxxx>
> > ---
> > xdiff/xutils.c | 4 ++--
> > 1 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/xdiff/xutils.c b/xdiff/xutils.c
> > index 04ad468..c6512a5 100644
> > --- a/xdiff/xutils.c
> > +++ b/xdiff/xutils.c
> > @@ -248,12 +248,12 @@ static unsigned long xdl_hash_record_with_whitespace(char const **data,
> > if (flags & XDF_IGNORE_WHITESPACE)
> > ; /* already handled */
> > else if (flags & XDF_IGNORE_WHITESPACE_CHANGE
> > - && ptr[1] != '\n') {
> > + && ptr[1] != '\n' && ptr + 1 < top) {
> > ha += (ha << 5);
> > ha ^= (unsigned long) ' ';
> > }
> > else if (flags & XDF_IGNORE_WHITESPACE_AT_EOL
> > - && ptr[1] != '\n') {
> > + && ptr[1] != '\n' && ptr + 1 < top) {
> > while (ptr2 != ptr + 1) {
> > ha += (ha << 5);
> > ha ^= (unsigned long) *ptr2;
>
> 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.
>
I appreciate your taking the time to look at the issue and explaining the
reasons for your change.
> -- >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@xxxxxxxxx>
Yeah... comparing this commit message to the original shows a pretty stark
difference. I'll get it 'the git way' eventually.
--
Thell
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html