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

Re: svn commit: r987869 - /subversion/branches/performance/subversion/libsvn_fs_fs/temp_serializer.c

From: Stefan Fuhrmann <stefanfuhrmann_at_alice-dsl.de>
Date: Wed, 25 Aug 2010 11:13:17 +0200

Stefan Sperling <stsp_at_elego.de
<mailto:stsp_at_elego.de?Subject=Re:%20svn%20commit:%20r987869%20-%20/subversion/branches/performance/subversion/libsvn_fs_fs/temp_serializer.c>>
wrote:
>
> On Sun, Aug 22, 2010 at 11:37:38AM -0000, stefan2_at_apache.org wrote:
> /> Author: stefan2 /
> /> Date: Sun Aug 22 11:37:38 2010 /
> /> New Revision: 987869 /
> /> /
> /> URL: http://svn.apache.org/viewvc?rev=987869&view=rev
> <http://svn.apache.org/viewvc?rev=987869&view=rev> /
> /> Log: /
> /> Fix VisualStudio build: memory size calculation for variable size /
> /> arrays is not portable. /
> /> /
> /> * subversion/libsvn_fs_fs/temp_serializer.c /
> /> (serialize_dir, serialize_txdelta_ops): fix array size calculation /
> /> /
> /> Modified: /
> />
> subversion/branches/performance/subversion/libsvn_fs_fs/temp_serializer.c
> /
> /> /
> /> Modified:
> subversion/branches/performance/subversion/libsvn_fs_fs/temp_serializer.c
> /
> /> URL:
> http://svn.apache.org/viewvc/subversion/branches/performance/subversion/libsvn_fs_fs/temp_serializer.c?rev=987869&r1=987868&r2=987869&view=diff
> <http://svn.apache.org/viewvc/subversion/branches/performance/subversion/libsvn_fs_fs/temp_serializer.c?rev=987869&r1=987868&r2=987869&view=diff>
> /
> />
> ==============================================================================
> /
> /> ---
> subversion/branches/performance/subversion/libsvn_fs_fs/temp_serializer.c
> (original) /
> /> +++
> subversion/branches/performance/subversion/libsvn_fs_fs/temp_serializer.c
> Sun Aug 22 11:37:38 2010 /
> /> @@ -272,7 +272,7 @@ serialize_dir(apr_hash_t *entries, apr_p /
> /> /
> /> /* calculate sizes */ /
> /> apr_size_t count = apr_hash_count(entries); /
> /> - apr_size_t entries_len = sizeof(svn_fs_dirent_t*[count]); /
> /> + apr_size_t entries_len = count * sizeof(svn_fs_dirent_t*[1]); /
>
> Do you really want the second asterisk in that line?
> It looks like you really want this:
>
> + apr_size_t entries_len = count * sizeof(svn_fs_dirent_t[1]);
>
No, it actually is an array of pointers. So, every element can be
serialized along with all its sub-elements as a single, contiguous
chunk of memory.
>
> /> /
> /> /* copy the hash entries to an auxilliary struct of known layout */ /
> /> hash_data.count = count; /
> /> @@ -422,7 +422,7 @@ serialize_txdelta_ops(svn_temp_serialize /
> /> /* the ops form a simple chunk of memory with no further references
> */ /
> /> svn_temp_serializer__push(context, /
> /> (const void * const *)ops, /
> /> - sizeof(svn_txdelta_op_t[count])); /
> /> + count * sizeof(svn_txdelta_op_t[1])); /
>
> Because you're not using the extra asterisk here, either.
>
I picked up the count*sizeof(T[1]) idiom from come STL implementations.
They were probably working around some non-standard compiler behavior
somewhere. Since that turned out to be unnecessary, I dropped the confusing
[1] in r988898.

-- Stefan^2.
Received on 2010-08-25 11:14:25 CEST

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