On 16.05.2011 13:49, Daniel Shahaf wrote:
stef...@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.
