On 16.05.2011 13:49, Daniel Shahaf wrote:
> stefan2_at_apache.org wrote on Sun, May 15, 2011 at 14:52:22 -0000:
>> Author: stefan2
>> Date: Sun May 15 14:52:22 2011
>> New Revision: 1103413
>>
>> URL: http://svn.apache.org/viewvc?rev=1103413&view=rev
>> Log:
>> If an in-place modification of some cache entry failed, we must remove
>> that entry because it might have become invalid or even corrupted.
>>
>> * subversion/libsvn_subr/cache-membuffer.c
>> (membuffer_cache_set_partial): drop the entry upon modification failure.
>>
>> Found by: danielsh
>>
>> Modified:
>> subversion/trunk/subversion/libsvn_subr/cache-membuffer.c
>>
>> Modified: subversion/trunk/subversion/libsvn_subr/cache-membuffer.c
>> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/cache-membuffer.c?rev=1103413&r1=1103412&r2=1103413&view=diff
>> ==============================================================================
>> --- subversion/trunk/subversion/libsvn_subr/cache-membuffer.c (original)
>> +++ subversion/trunk/subversion/libsvn_subr/cache-membuffer.c Sun May 15 14:52:22 2011
>> @@ -1331,36 +1331,47 @@ membuffer_cache_set_partial(svn_membuffe
>> */
>> err = func(&data,&size, baton, pool);
>>
>> - /* if modification caused a re-allocation, we need to remove the old
>> - * entry and to copy the new data back into cache.
>> - */
>> - if (data != orig_data)
>> + if (err)
>> {
>> - /* Remove the old entry and try to make space for the new one.
>> + /* Something somewhere when wrong while FUNC was modifying the
>> + * changed item. Thus, it might have become invalid /corrupted.
>> + * We better drop that.
>> */
>> drop_entry(cache, entry);
>> - if ( (cache->data_size / 4> size)
>> -&& ensure_data_insertable(cache, size))
>> + }
>> + else
>> + {
>> + /* if the modification caused a re-allocation, we need to remove
>> + * the old entry and to copy the new data back into cache.
>> + */
>> + if (!err&& (data != orig_data))
> The "!err" part is unnecessary.
>
> It seems you could do:
>
> if (err)
> else if (data != orig_data)
That was an artifact from intermediate code. Fixed in r1104319.
Thanks for all the reviews. I kinda start relying on them ;)
-- Stefan^2.
Received on 2011-05-17 17:17:48 CEST