On 14 May 2015 at 15:37, Branko Čibej <br...@wandisco.com> wrote:
> On 13.05.2015 19:04, Ivan Zhakov wrote:
>> [adjusting subject to make it valid vote thread]
>>
>> On 13 May 2015 at 19:23, Stefan Fuhrmann <stefan.fuhrm...@wandisco.com>
>> wrote:
>>> Hi there,
>>>
>>> Ivan has reviewed my recent membuffer cache
>>> key handling changes, corrected and backported
>>> them on the 1.9-cache-improvements branch.
>>>
>>> I reviewed it and I'm +1 on merging it to /trunk -
>>> hoping we may even get it into 1.9. Since this
>>> touches a sensitive part of the server code, I'd
>>> like to see 2 more +1s for the branch->/trunk
>>> merge.
>>>
>> +1.
>>
>> PS: I think detailed log message will be useful for reviewers. I'll
>> make it tomorrow if you didn't outstrip me.
>
> -0 because:
>
> $ make
> .../subversion/libsvn_subr/cache-membuffer.c:2626:59: warning: implicit
> conversion loses integer
> precision: 'unsigned long' to 'apr_uint32_t' (aka 'unsigned int')
> [-Wshorten-64-to-32]
> cache->combined_key.entry_key.key_len = aligned_key_len + prefix_len;
> ~ ~~~~~~~~~~~~~~~~^~~~~~~~~~~~
> .../subversion/libsvn_subr/cache-membuffer.c:2664:58: warning: implicit
> conversion loses integer
> precision: 'unsigned long' to 'apr_uint32_t' (aka 'unsigned int')
> [-Wshorten-64-to-32]
> cache->combined_key.entry_key.key_len = prefix_len + 16;
> ~ ~~~~~~~~~~~^~~~
> .../subversion/libsvn_subr/cache-membuffer.c:3161:37: warning: implicit
> conversion loses integer
> precision: 'apr_size_t' (aka 'unsigned long') to 'apr_uint32_t' (aka
> 'unsigned int') [-Wshorten-64-to-32]
> cache->prefix.entry_key.key_len = prefix_len;
> ~ ^~~~~~~~~~
> 3 warnings generated.
>
> +1 if these warnings get fixed before or as part of the merge without
> adding casts.
>
I suggest attached patch to resolve these warnings.
Log message:
[[[
* subversion/libsvn_subr/cache-membuffer.c
(entry_key_t): Change type of KEY_LEN field to apr_size_t instead of
apr_uint32_t.
]]]
--
Ivan Zhakov
Index: subversion/libsvn_subr/cache-membuffer.c
===================================================================
--- subversion/libsvn_subr/cache-membuffer.c (revision 1679357)
+++ subversion/libsvn_subr/cache-membuffer.c (working copy)
@@ -197,7 +197,7 @@
/* Length of the full key. This value is aligned to ITEM_ALIGNMENT to
* make sure the subsequent item content is properly aligned. */
- apr_uint32_t key_len;
+ apr_size_t key_len;
} entry_key_t;
/* A full key, i.e. the combination of the cache's key prefix with some
