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

Re: Bugs in 1.8.5 libsvn_subr which make 32-bit threaded svnserve sometimes hang and eat 100% cpu

From: James McCoy <jamessan_at_debian.org>
Date: Thu, 13 Feb 2014 07:41:10 -0500

On Thu, Feb 13, 2014 at 01:29:25PM +0100, Stefan Sperling wrote:
> On Thu, Feb 13, 2014 at 03:49:46PM +0400, vitalif_at_yourcmc.ru wrote:
> > --- a/subversion/libsvn_subr/cache-membuffer.c 2014-02-12 21:42:12.483208244 +0000
> > +++ b/subversion/libsvn_subr/cache-membuffer.c 2014-02-12 21:45:54.275215290 +0000
> > @@ -1374,7 +1374,9 @@ membuffer_cache_set_internal(svn_membuff
> > * the old spot, just re-use that space. */
> > if (entry && ALIGN_VALUE(entry->size) >= size && buffer)
> > {
> > - cache->data_used += size - entry->size;
> > + /* not "+=" because (size - entry_size) is almost always a big 32-bit
> > + unsigned representation of a negative value on 32-bit platforms */
> > + cache->data_used = cache->data_used + size - entry->size;
> > entry->size = size;
> >
> > #ifdef SVN_DEBUG_CACHE_MEMBUFFER
>
> Style nit: I believe the above comment will be much less clear when read
> without its preceding context line in the diff, because the reader won't
> have an obvious way of connecting the reference to "+=" to anything.

How about casting size to apr_uint64_t instead? That allows using the
previous += since the upcast happens on an operand instead of the result
and is a bit more self-documenting.

   cache->data_used += (apr_uint64_t)size - entry->size;

Cheers,

-- 
James
GPG Key: 4096R/331BA3DB 2011-12-05 James McCoy <jamessan_at_debian.org>

Received on 2014-02-13 13:41:48 CET

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.