James McCoy wrote: > On Thu, Feb 13, 2014 at 01:29:25PM +0100, Stefan Sperling wrote: >> On Thu, Feb 13, 2014 at 03:49:46PM +0400, vita...@yourcmc.ru wrote: >>> - 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; >> >> 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;
+1. But the most important thing is: Thank you, Vitaliy, for finding and fixing these bugs! - Julian
