[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: Keywords as hash: API design: null or empty hash

From: Daniel Rall <dlr_at_finemaltcoding.com>
Date: 2005-09-22 20:40:04 CEST

On Thu, 22 Sep 2005, Julian Foad wrote:
...
> That said, I tentatively suggest the attached patch. What do you think of
> it?

Looks good! A few comments inlined below.

- Dan

...
> [[[
> Make the hash-based keywords-processing functions handle an empty hash
> efficiently.
>
> * subversion/libsvn_subr/subst.c
> (svn_subst_translate_stream3,
> svn_subst_translate_cstring2,
> svn_subst_copy_and_translate3):
> Treat an empty hash in the same way as a null pointer, thus
> avoiding the overhead of doing a no-op translation.
> ]]]
...
> --- subversion/libsvn_subr/subst.c (revision 16194)
> +++ subversion/libsvn_subr/subst.c (working copy)
> @@ -797,6 +797,10 @@ svn_subst_translate_stream3 (svn_stream_
>
> buf = apr_palloc (pool, SVN_STREAM_CHUNK_SIZE + 1);
>
> + /* For efficiency, convert an empty set of keywords to NULL. */
> + if (keywords && (apr_hash_count (keywords) == 0))

Parens around the second condition are extraneous.

> + keywords = NULL;
> +
> /* The docstring requires that *some* translation be requested. */
> assert (eol_str || keywords);
> interesting = (eol_str && keywords) ? "$\r\n" : eol_str ? "\r\n" : "$";
> @@ -952,7 +956,7 @@ svn_subst_translate_cstring2 (const char
> src_stringbuf = svn_stringbuf_create (src, pool);
>
> /* The easy way out: no translation needed, just copy. */
> - if (! (eol_str || keywords))
> + if (! (eol_str || (keywords && apr_hash_count (keywords))))

For consistency with the check "== 0" -- and to improve clarity by being more
explicit -- the return value of apr_hash_count() should be checked as "> 0".

> {
> dst_stringbuf = svn_stringbuf_dup (src_stringbuf, pool);
> goto all_good;
> @@ -1194,7 +1198,7 @@ svn_subst_copy_and_translate3 (const cha
> }
>
> /* The easy way out: no translation needed, just copy. */
> - if (! (eol_str || keywords))
> + if (! (eol_str || (keywords && apr_hash_count (keywords))))

Ditto.

> return svn_io_copy_file (src, dst, FALSE, pool);
>
> subpool = svn_pool_create (pool);

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Sep 22 20:42:00 2005

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.