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

Re: Crashes in 1.8.0 test suite on Solaris Sparc (wrong alignment in cache_lookup())

From: Rainer Jung <rainer.jung_at_kippdata.de>
Date: Thu, 20 Jun 2013 18:54:55 +0200

One correction see below.

On 20.06.2013 18:02, Mattias EngdegŚrd wrote:
> 20 jun 2013 kl. 16.54 skrev Ivan Zhakov:
>
>> It seems the code is missing #ifdef SVN_UNALIGNED_ACCESS_IS_OK . The
>> attached patch should fix problem, but I'm not sure that this is right
>> solution for the problem.
>
> Assuming the hash function does not need to be portable (which appears
> likely from a cursory look at the code), that fix is basically correct,
> if you move out the assignment i=0 to right before the section you made
> conditional. The following loop will complete the hash.
> In other words, something like:
>
> Index: subversion/libsvn_fs_fs/tree.c
> ===================================================================
> --- subversion/libsvn_fs_fs/tree.c (revision 1494562)
> +++ subversion/libsvn_fs_fs/tree.c (working copy)
> @@ -353,8 +353,11 @@
>
> /* need to do a full lookup. Calculate the hash value
> (HASH_VALUE has been initialized to REVISION). */
> + i = 0;
> +#ifdef SVN_UNALIGNED_ACCESS_IS_OK

   ^^^^^^ #if please. The macro is always "0" or "1".

> - for (i = 0; i + 4 <= path_len; i += 4)
> + for (; i + 4 <= path_len; i += 4)
> hash_value = hash_value * 0xd1f3da69 + *(const apr_uint32_t*)(path
> + i);
> +#endif
>
> for (; i < path_len; ++i)
> hash_value = hash_value * 33 + path[i];
>
> (By the way, that code violates C's aliasing rules, but we probably get
> away with it in this case.)
Received on 2013-06-20 18:55:47 CEST

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