You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2012/03/03 11:53:16 UTC
svn commit: r1296596 - /subversion/trunk/subversion/libsvn_delta/xdelta.c
Author: stefan2
Date: Sat Mar 3 10:53:16 2012
New Revision: 1296596
URL: http://svn.apache.org/viewvc?rev=1296596&view=rev
Log:
* subversion/libsvn_delta/xdelta.c
(reverse_match_length): actually return MAX_LEN if MAX_LEN chars match.
Found by: julianfoad
Modified:
subversion/trunk/subversion/libsvn_delta/xdelta.c
Modified: subversion/trunk/subversion/libsvn_delta/xdelta.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_delta/xdelta.c?rev=1296596&r1=1296595&r2=1296596&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_delta/xdelta.c (original)
+++ subversion/trunk/subversion/libsvn_delta/xdelta.c Sat Mar 3 10:53:16 2012
@@ -260,11 +260,15 @@ reverse_match_length(const char *a, cons
#endif
+ /* If we find a mismatch at -pos, pos-1 characters matched.
+ */
while (++pos <= max_len)
if (a[0-pos] != b[0-pos])
- break;
+ return pos - 1;
- return pos-1;
+ /* No mismatch found -> at least MAX_LEN machting chars.
+ */
+ return max_len;
}
Re: svn commit: r1296596 - /subversion/trunk/subversion/libsvn_delta/xdelta.c
Posted by Julian Foad <ju...@btopenworld.com>.
Stefan Fuhrmann wrote:
> Julian Foad wrote:
>> Please edit the log message for this rev.
> Done for r1296596.
Hi Stefan. Thank you for removing "Found by: julianfoad", but also you should change the log message from the present
"actually return MAX_LEN if MAX_LEN chars match."
to something like
"Add comments and re-write code for clarity. No functional change."
as r1296596 caused no change in return values.
>> (I assume you'll revisit this soon, as my original comment still
>> stands. Sorry if it was confusing. What I meant, basically, is
>> that the function doesn't return what the doc string says it will
>> return, AFAICT. Quite likely it's the doc string that's wrong.)
>
> r1310770 should address that issue now.
Yup, I agree that makes the function's doc string match its behaviour. Thanks.
- Julian
Re: svn commit: r1296596 - /subversion/trunk/subversion/libsvn_delta/xdelta.c
Posted by Stefan Fuhrmann <eq...@web.de>.
Julian Foad wrote:
> Hi Stefan.
>
> Please edit the log message for this rev.
Done for r1296596.
> (I assume you'll revisit this soon, as my original comment still stands. Sorry if it was confusing. What I meant, basically, is that the function doesn't return what the doc string says it will return, AFAICT. Quite likely it's the doc string that's wrong.)
>
> - Julian
r1310770 should address that issue now.
-- Stefan^2.
>
>
> ----- Original Message -----
>> From: Stefan Fuhrmann<st...@alice-dsl.de>
>> To:
>> Cc: dev@subversion.apache.org
>> Sent: Sunday, 4 March 2012, 23:48
>> Subject: Re: svn commit: r1296596 - /subversion/trunk/subversion/libsvn_delta/xdelta.c
>>
>> On 04.03.2012 11:42, Daniel Shahaf wrote:
>>> stefan2@apache.org wrote on Sat, Mar 03, 2012 at 10:53:16 -0000:
>>>> Author: stefan2
>>>> Date: Sat Mar 3 10:53:16 2012
>>>> New Revision: 1296596
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1296596&view=rev
>>>> Log:
>>>> * subversion/libsvn_delta/xdelta.c
>>>> (reverse_match_length): actually return MAX_LEN if MAX_LEN chars
>> match.
>>>> Found by: julianfoad
>>>>
>>>> Modified:
>>>> subversion/trunk/subversion/libsvn_delta/xdelta.c
>>>>
>>>> Modified: subversion/trunk/subversion/libsvn_delta/xdelta.c
>>>> URL:
>> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_delta/xdelta.c?rev=1296596&r1=1296595&r2=1296596&view=diff
>> ==============================================================================
>>>> --- subversion/trunk/subversion/libsvn_delta/xdelta.c (original)
>>>> +++ subversion/trunk/subversion/libsvn_delta/xdelta.c Sat Mar 3
>> 10:53:16 2012
>>>> @@ -260,11 +260,15 @@ reverse_match_length(const char *a, cons
>>>>
>>>> #endif
>>>>
>>>> + /* If we find a mismatch at -pos, pos-1 characters matched.
>>>> + */
>>>> while (++pos<= max_len)
>>>> if (a[0-pos] != b[0-pos])
>>>> - break;
>>>> + return pos - 1;
>>>>
>>>> - return pos-1;
>>>> + /* No mismatch found -> at least MAX_LEN machting chars.
>>>> + */
>>>> + return max_len;
>>> I may be blind, but isn't the code before this diff and after it
>>> equivalent?
>>>
>>> Both the old and new code return POS-1 when the if() statement is
>>> entered, and if the code falls off the end of the while() loop then
>>> necessarily POS=1+MAX_LEN, again meaning that the old and new code are
>>> equivalent.
>> You are right. It's been too early in the morning
>> for me and Julian's comment got me confused.
>> But at least, the code slightly clearer now.
>>
>> -- Stefan^2.
>>
Re: svn commit: r1296596 - /subversion/trunk/subversion/libsvn_delta/xdelta.c
Posted by Julian Foad <ju...@btopenworld.com>.
Hi Stefan.
Please edit the log message for this rev.
(I assume you'll revisit this soon, as my original comment still stands. Sorry if it was confusing. What I meant, basically, is that the function doesn't return what the doc string says it will return, AFAICT. Quite likely it's the doc string that's wrong.)
- Julian
----- Original Message -----
> From: Stefan Fuhrmann <st...@alice-dsl.de>
> To:
> Cc: dev@subversion.apache.org
> Sent: Sunday, 4 March 2012, 23:48
> Subject: Re: svn commit: r1296596 - /subversion/trunk/subversion/libsvn_delta/xdelta.c
>
> On 04.03.2012 11:42, Daniel Shahaf wrote:
>> stefan2@apache.org wrote on Sat, Mar 03, 2012 at 10:53:16 -0000:
>>> Author: stefan2
>>> Date: Sat Mar 3 10:53:16 2012
>>> New Revision: 1296596
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1296596&view=rev
>>> Log:
>>> * subversion/libsvn_delta/xdelta.c
>>> (reverse_match_length): actually return MAX_LEN if MAX_LEN chars
> match.
>>>
>>> Found by: julianfoad
>>>
>>> Modified:
>>> subversion/trunk/subversion/libsvn_delta/xdelta.c
>>>
>>> Modified: subversion/trunk/subversion/libsvn_delta/xdelta.c
>>> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_delta/xdelta.c?rev=1296596&r1=1296595&r2=1296596&view=diff
>>>
> ==============================================================================
>>> --- subversion/trunk/subversion/libsvn_delta/xdelta.c (original)
>>> +++ subversion/trunk/subversion/libsvn_delta/xdelta.c Sat Mar 3
> 10:53:16 2012
>>> @@ -260,11 +260,15 @@ reverse_match_length(const char *a, cons
>>>
>>> #endif
>>>
>>> + /* If we find a mismatch at -pos, pos-1 characters matched.
>>> + */
>>> while (++pos<= max_len)
>>> if (a[0-pos] != b[0-pos])
>>> - break;
>>> + return pos - 1;
>>>
>>> - return pos-1;
>>> + /* No mismatch found -> at least MAX_LEN machting chars.
>>> + */
>>> + return max_len;
>> I may be blind, but isn't the code before this diff and after it
>> equivalent?
>>
>> Both the old and new code return POS-1 when the if() statement is
>> entered, and if the code falls off the end of the while() loop then
>> necessarily POS=1+MAX_LEN, again meaning that the old and new code are
>> equivalent.
>
> You are right. It's been too early in the morning
> for me and Julian's comment got me confused.
> But at least, the code slightly clearer now.
>
> -- Stefan^2.
>
Re: svn commit: r1296596 - /subversion/trunk/subversion/libsvn_delta/xdelta.c
Posted by Stefan Fuhrmann <st...@alice-dsl.de>.
On 04.03.2012 11:42, Daniel Shahaf wrote:
> stefan2@apache.org wrote on Sat, Mar 03, 2012 at 10:53:16 -0000:
>> Author: stefan2
>> Date: Sat Mar 3 10:53:16 2012
>> New Revision: 1296596
>>
>> URL: http://svn.apache.org/viewvc?rev=1296596&view=rev
>> Log:
>> * subversion/libsvn_delta/xdelta.c
>> (reverse_match_length): actually return MAX_LEN if MAX_LEN chars match.
>>
>> Found by: julianfoad
>>
>> Modified:
>> subversion/trunk/subversion/libsvn_delta/xdelta.c
>>
>> Modified: subversion/trunk/subversion/libsvn_delta/xdelta.c
>> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_delta/xdelta.c?rev=1296596&r1=1296595&r2=1296596&view=diff
>> ==============================================================================
>> --- subversion/trunk/subversion/libsvn_delta/xdelta.c (original)
>> +++ subversion/trunk/subversion/libsvn_delta/xdelta.c Sat Mar 3 10:53:16 2012
>> @@ -260,11 +260,15 @@ reverse_match_length(const char *a, cons
>>
>> #endif
>>
>> + /* If we find a mismatch at -pos, pos-1 characters matched.
>> + */
>> while (++pos<= max_len)
>> if (a[0-pos] != b[0-pos])
>> - break;
>> + return pos - 1;
>>
>> - return pos-1;
>> + /* No mismatch found -> at least MAX_LEN machting chars.
>> + */
>> + return max_len;
> I may be blind, but isn't the code before this diff and after it
> equivalent?
>
> Both the old and new code return POS-1 when the if() statement is
> entered, and if the code falls off the end of the while() loop then
> necessarily POS=1+MAX_LEN, again meaning that the old and new code are
> equivalent.
You are right. It's been too early in the morning
for me and Julian's comment got me confused.
But at least, the code slightly clearer now.
-- Stefan^2.
Re: svn commit: r1296596 -
/subversion/trunk/subversion/libsvn_delta/xdelta.c
Posted by Daniel Shahaf <da...@elego.de>.
stefan2@apache.org wrote on Sat, Mar 03, 2012 at 10:53:16 -0000:
> Author: stefan2
> Date: Sat Mar 3 10:53:16 2012
> New Revision: 1296596
>
> URL: http://svn.apache.org/viewvc?rev=1296596&view=rev
> Log:
> * subversion/libsvn_delta/xdelta.c
> (reverse_match_length): actually return MAX_LEN if MAX_LEN chars match.
>
> Found by: julianfoad
>
> Modified:
> subversion/trunk/subversion/libsvn_delta/xdelta.c
>
> Modified: subversion/trunk/subversion/libsvn_delta/xdelta.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_delta/xdelta.c?rev=1296596&r1=1296595&r2=1296596&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_delta/xdelta.c (original)
> +++ subversion/trunk/subversion/libsvn_delta/xdelta.c Sat Mar 3 10:53:16 2012
> @@ -260,11 +260,15 @@ reverse_match_length(const char *a, cons
>
> #endif
>
> + /* If we find a mismatch at -pos, pos-1 characters matched.
> + */
> while (++pos <= max_len)
> if (a[0-pos] != b[0-pos])
> - break;
> + return pos - 1;
>
> - return pos-1;
> + /* No mismatch found -> at least MAX_LEN machting chars.
> + */
> + return max_len;
I may be blind, but isn't the code before this diff and after it
equivalent?
Both the old and new code return POS-1 when the if() statement is
entered, and if the code falls off the end of the while() loop then
necessarily POS=1+MAX_LEN, again meaning that the old and new code are
equivalent.
> }
>
>
>
>
Re: svn commit: r1296596 -
/subversion/trunk/subversion/libsvn_delta/xdelta.c
Posted by Daniel Shahaf <da...@elego.de>.
stefan2@apache.org wrote on Sat, Mar 03, 2012 at 10:53:16 -0000:
> Author: stefan2
> Date: Sat Mar 3 10:53:16 2012
> New Revision: 1296596
>
> URL: http://svn.apache.org/viewvc?rev=1296596&view=rev
> Log:
> * subversion/libsvn_delta/xdelta.c
> (reverse_match_length): actually return MAX_LEN if MAX_LEN chars match.
>
> Found by: julianfoad
>
> Modified:
> subversion/trunk/subversion/libsvn_delta/xdelta.c
>
> Modified: subversion/trunk/subversion/libsvn_delta/xdelta.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_delta/xdelta.c?rev=1296596&r1=1296595&r2=1296596&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_delta/xdelta.c (original)
> +++ subversion/trunk/subversion/libsvn_delta/xdelta.c Sat Mar 3 10:53:16 2012
> @@ -260,11 +260,15 @@ reverse_match_length(const char *a, cons
>
> #endif
>
> + /* If we find a mismatch at -pos, pos-1 characters matched.
> + */
> while (++pos <= max_len)
> if (a[0-pos] != b[0-pos])
> - break;
> + return pos - 1;
>
> - return pos-1;
> + /* No mismatch found -> at least MAX_LEN machting chars.
> + */
> + return max_len;
I may be blind, but isn't the code before this diff and after it
equivalent?
Both the old and new code return POS-1 when the if() statement is
entered, and if the code falls off the end of the while() loop then
necessarily POS=1+MAX_LEN, again meaning that the old and new code are
equivalent.
> }
>
>
>
>