Lieven Govaerts wrote:
>>From: Julian Foad [mailto:julianfoad@btopenworld.com]
>
>>> if (b->keyword_off > SVN_KEYWORD_MAX_LEN ||
>>
>>Should that be ">="? By the time it's greater, we'll already
>>have written at least one character off the end of the buffer.
>
> I don't think so. b-keyword_off was post-incremented in the previous line,
> so we can't write off the end of the buffer.
If it's MAX+1 at this point, then the character was written into buffer[MAX],
which is off the end of the buffer:
char keyword_buf[SVN_KEYWORD_MAX_LEN];
b->keyword_buf[b->keyword_off++] = *p++;
Yes? (I notice that the "keyword_name" buffer has a length of MAX+1, but
"keyword_buf" doesn't. I don't think keyword_name needs to be that long; I
think MAX-2 characters, plus one for a null, is enough.)
>>Check what's going to happen at the boundary condition: will
>>it do the right thing for a valid keyword-pattern of exactly
>>MAX_LEN? MAX_LEN-1? MAX_LEN+1?
>>What about a non-keyword pattern of those lengths?
>
> I've added lines 76-82 in translate-test.c for testing for these border
> cases. They all seem to work as expected.
Thanks, but I don't think the too-long patterns (lines 81 and 82) are caught by
your code, they're caught by the subsequent existing test (because a non-'$'
character appears at position MAX). I think you need dollar signs in the
254th/255th/256th positions so that your code is executed at those positions.
Something like:
$Author: aaaaaa$$$ $
... where the length up to the end of "$$$" is 254/255/256.
>>I think "b->keyword_off >= SVN_KEYWORD_MAX_LEN" will need to
>>go after "translate_keyword()", but see what you think.
>
> No problem with that.
>
> However, I don't see a change in behaviour when I implement the >= change
> nor the line-order change.
This is what I expect:
If we change ">" to ">=" only, then a keyword of length exactly MAX will not be
processed because "off >= MAX" will be true and will prevent the
"translate_keyword" call.
If we change ">" to ">=" and move the test to after the "translate_keyword"
call, then the keyword of length MAX will be processed.
> The new tests should have captured this change. I
> guess that b->keyword_off == SVN_KEYWORD_MAX_LEN will not be reached there
> because it's already handled earlier in the else block at line 998.
> The attached patch is based on yours and includes these two changes and some
> extra tests, no new changes in the algorithm itself.
Thanks. Let me know what you think about the above.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Feb 14 23:52:20 2006