Julian,
attached you'll find a new patch for issue 1780, that I think adresses your
remarks.
> -----Original Message-----
> From: Julian Foad [mailto:julianfoad@btopenworld.com]
> Sent: zondag 12 februari 2006 21:14
>
> More precisely, it solves the problem of user names or URLs
> containing dollar signs, unless a dollar sign is the first
> character or follows a space.
>
> I think this level of functionality is worth having, and I
> would commit it except that I have two concerns with the
> implementation.
>
> 1) It fails (with an assert) when searching for a keyword in
> a line which is longer than SVN_KEYWORD_MAX_LEN. Although we
> don't support arbitrarily long keywords, we must continue to
> support arbitrarily long lines. To reproduce, just let it
> try keyword translation on a line consisting of "$" plus (MAX
> - 2) spaces plus "$$".
I added line 76 in translate-test.c to test for this problem. Solved it in
the attached patch.
> 2) It takes order(N-squared) time to process a line
> containing N dollar signs.
> On my system I found many text files with more than 25
> dollar signs in the same line, including:
>
> * Text representations of binary data, e.g.
> /etc/X11/xdm/pixmaps/xorg.xpm
> * Shell scripts, makefiles, etc., e.g. subversion/build/libtool.m4
> * Config files referencing env-vars, e.g. .kde/share/config/kdeglobals
> * Our test for cases like this:
> subversion/tests/libsvn_wc/translate-test.c
>
> I wouldn't think it strange for someone to use a line of
> about eighty dollar signs to separate sections in a text
> file. Processing ten lines, each consisting of 250 dollar
> signs, took over a second on my rather slow system.
> Certainly that's not a huge amount of time for a rare case,
> but we ought to cope well with unusual cases if we can.
I tried to improve performance, by now only using the matching algorithm
when a valid keyword has been found. So it will only skip dollar signs when
a keyword was matched first. Whenever something like '$testblahblah$' is
found, it'll just skip that part of text.
I had to separate the keyword matching functionality in translate_keyword in
its own function match_keyword, to support this performance improvement.
Since these are internal functions and not used outside subst.c this is no
problem. Correct assumption?
That makes the patch a bit larger, and maybe a bit less clear.
Log message:
[[[
Fix issue #1780: correct keyword expansion when URL or Author value contains
'$'.
* subversion/libsvn_subr/subst.c
(translate_chunk): add algorithm that keeps searching for next '$' in the
value of a valid expanded keyword until a correct value is found or search
limits are reached.
(match_keyword): new local function that matches a valid keyword in a char
buffer.
(translate_keyword): added keyword_name parameter, translate a keyword
keyword_name with its value. Keyword matching was put in match_keyword.
* subversion/tests/libsvn_wc/translate-test.c
(lines[]): added lines 74-76 to test on '$' in Author and URL keywords.
]]]
> I note that "svn status" processes the locally-modified file
> every time I run "svn status"; that's a separate problem that
> we might want to look at.
>
> The bug (1) should be straightforward to fix. I'm not sure
> what to say about the efficiency issue (2). I'd very much
> like to see it improved, but I might not insist on it.
>
> - Julian
Lieven.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Feb 14 00:44:33 2006