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) > { > - /* Write the new entry. > + /* Remove the old entry and try to make space for the new one. > */ > - entry->size = size; > - entry->offset = cache->current_data; > - if (size) > - memcpy(cache->data + entry->offset, data, size); > - > - /* Link the entry properly. > - */ > - insert_entry(cache, entry); > + drop_entry(cache, entry); > + if ( (cache->data_size / 4 > size) > + && ensure_data_insertable(cache, size)) > + { > + /* Write the new entry. > + */ > + entry->size = size; > + entry->offset = cache->current_data; > + if (size) > + memcpy(cache->data + entry->offset, data, size); > + > + /* Link the entry properly. > + */ > + insert_entry(cache, entry); > > #ifdef DEBUG_CACHE_MEMBUFFER > > - /* Remember original content, type and key (hashes) > - */ > - SVN_ERR(store_content_part(tag, data, size, pool)); > - memcpy(&entry->tag, tag, sizeof(*tag)); > + /* Remember original content, type and key (hashes) > + */ > + SVN_ERR(store_content_part(tag, data, size, pool)); > + memcpy(&entry->tag, tag, sizeof(*tag)); > > #endif > + } > } > } > } > >