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

Re: svn commit: r1103413 - /subversion/trunk/subversion/libsvn_subr/cache-membuffer.c

From: Daniel Shahaf <d.s_at_daniel.shahaf.name>
Date: Mon, 16 May 2011 13:49:52 +0200

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)

> {
> - /* 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
> + }
> }
> }
> }
>
>
Received on 2011-05-16 13:50:29 CEST

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.