Re: Help/Advice needed on diff bug in xutils.c

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Tue, 4 Aug 2009, Thell Fowler wrote:

> There is a bug in git diff (ignoring whitespace) that does not take into 
> account a trailing space at the end of a line at the end of a file when 
> no new line follows.
> 
> Here is the example of the bug:
> mkdir test_ws_eof
> cd test_ws_eof
> git init
> echo -n "Test" > test.txt
> git add .
> git commit -m'test'
> git symbolic-ref HEAD refs/heads/with_space
> rm .git/index
> git clean -f
> echo -n "Test ">test.txt
> git add .
> git commit -m'test'
> # Ignoring all whitespace there shouldn't be a diff.
> git diff -w master -- test.txt
> # Ignoring space at eol there shouldn't be a diff
> git diff --ignore-space-at-eol master -- test.txt
> # Ignoring with -b might have a case for a diff showing.
> git diff -b master -- test.txt

If you turn that into a patch to, say, t/t4015-diff-whitespace.sh (adding 
a test_expect_failure for a known bug), it is much easier to convince 
developers to work on the issue.

> In the xutils.c xdl_hash_record_with_whitespace function the trailing 
> space prior to eof was being calculated into the hash, I fixed that with 
> the change below, but there is still a difference being noted in 
> xdl_recmatch because of the size difference.
> 
> Before I go changing something that shouldn't be changed could someone
> provide some input please?
> 
> Thanks for reading,
> Thell
> 
> diff --git a/xdiff/xutils.c b/xdiff/xutils.c
> index 04ad468..623da92 100644
> --- a/xdiff/xutils.c
> +++ b/xdiff/xutils.c
> @@ -243,17 +243,17 @@ static unsigned long
> xdl_hash_record_with_whitespace(char
> const **data,
>                 if (isspace(*ptr)) {
>                         const char *ptr2 = ptr;
>                         while (ptr + 1 < top && isspace(ptr[1])
> -                                       && ptr[1] != '\n')
> +                                       && ( ptr[1] != '\n' && ptr[1] !=
> '\0' ) )

First, your coding style is different from the surrounding code.  I think 
it goes without saying that this should be fixed.

Second, you do not need the parentheses at all (and therefore they should 
go).

Third, libxdiff does not assume to be fed NUL delimited strings.

Fourth, that condition "ptr + 1 < top" is already doing what you tried to 
accomplish here.

So I guess that you need to do add "ptr + 1 < top" checks 
instead.

Thanks,
Dscho

--
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

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]