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 16:30:13 UTC
svn commit: r1296640 - /subversion/trunk/subversion/libsvn_repos/rev_hunt.c
Author: stefan2
Date: Sat Mar 3 15:30:12 2012
New Revision: 1296640
URL: http://svn.apache.org/viewvc?rev=1296640&view=rev
Log:
* subversion/libsvn_repos/rev_hunt.c
(svn_repos_get_committed_info): use sizeof() instead of magic numbers
Suggested by: danielsh
Modified:
subversion/trunk/subversion/libsvn_repos/rev_hunt.c
Modified: subversion/trunk/subversion/libsvn_repos/rev_hunt.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/rev_hunt.c?rev=1296640&r1=1296639&r2=1296640&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_repos/rev_hunt.c (original)
+++ subversion/trunk/subversion/libsvn_repos/rev_hunt.c Sat Mar 3 15:30:12 2012
@@ -170,12 +170,12 @@ svn_repos_get_committed_info(svn_revnum_
SVN_ERR(svn_fs_revision_proplist(&revprops, fs, *committed_rev, pool));
/* Extract date and author from these revprops. */
- committed_date_s = apr_hash_get(revprops,
- SVN_PROP_REVISION_DATE,
- 8);
- last_author_s = apr_hash_get(revprops,
- SVN_PROP_REVISION_AUTHOR,
- 10);
+ committed_date_s = apr_hash_get(revprops,
+ SVN_PROP_REVISION_DATE,
+ sizeof(SVN_PROP_REVISION_DATE)-1);
+ last_author_s = apr_hash_get(revprops,
+ SVN_PROP_REVISION_AUTHOR,
+ sizeof(SVN_PROP_REVISION_AUTHOR)-1);
*committed_date = committed_date_s ? committed_date_s->data : NULL;
*last_author = last_author_s ? last_author_s->data : NULL;
Re: svn commit: r1296640 - use sizeof() instead of magic numbers
Posted by Stefan Fuhrmann <eq...@web.de>.
On 06.03.2012 12:48, Julian Foad wrote:
> stefan2@apache.org wrote:
>
>> URL: http://svn.apache.org/viewvc?rev=1296640&view=rev
>> Log:
>> * subversion/libsvn_repos/rev_hunt.c
>> (svn_repos_get_committed_info): use sizeof() instead of magic numbers
>> - committed_date_s = apr_hash_get(revprops,
>> - SVN_PROP_REVISION_DATE,
>> - 8);
> [...]
>> + committed_date_s = apr_hash_get(revprops,
>> + SVN_PROP_REVISION_DATE,
>> + sizeof(SVN_PROP_REVISION_DATE)-1);
> [...]
>
> Consider using the special value "APR_HASH_KEY_STRING". Last time I looked at the APR hash source code, it looked like the implementation was (as far as I could see by inspection) approximately just as fast with "APR_HASH_KEY_STRING" as with passing the specific length. Using it has the benefit of simplicity for the developers, reducing the opportunity for mistakes such as the one fixed by r1242353 (which is merely an example of an unnoticed typo in repetitive code, nothing to do with hash keys).
Hi Julian,
I'm aware of APR_HASH_KEY_STRING and APR's default-
hash function implementation.
A compatible, optimized variant of that sits in my commit queue
next to a ton of other things. In that code, the fixed-size variant
is about twice as fast as the variable-size one.
-- Stefan^2.
Re: svn commit: r1296640 - use sizeof() instead of magic numbers
Posted by Julian Foad <ju...@btopenworld.com>.
stefan2@apache.org wrote:
> URL: http://svn.apache.org/viewvc?rev=1296640&view=rev
> Log:
> * subversion/libsvn_repos/rev_hunt.c
> (svn_repos_get_committed_info): use sizeof() instead of magic numbers
> - committed_date_s = apr_hash_get(revprops,
> - SVN_PROP_REVISION_DATE,
> - 8);
[...]
> + committed_date_s = apr_hash_get(revprops,
> + SVN_PROP_REVISION_DATE,
> + sizeof(SVN_PROP_REVISION_DATE)-1);
[...]
Consider using the special value "APR_HASH_KEY_STRING". Last time I looked at the APR hash source code, it looked like the implementation was (as far as I could see by inspection) approximately just as fast with "APR_HASH_KEY_STRING" as with passing the specific length. Using it has the benefit of simplicity for the developers, reducing the opportunity for mistakes such as the one fixed by r1242353 (which is merely an example of an unnoticed typo in repetitive code, nothing to do with hash keys).
- Julian