[replying to myself - never a good sign on Friday afternoon]
On 01/28/2011 02:59 PM, Eric Blake wrote:
> +/* divide value by size, rounding up */
> +# define VIR_DIV_UP(value, size) (((value) + (size) - 1) / (size))
Hmm - now that we are codifying it, can we make it less prone to
overflow issues?
ACK to everything else, but let's make sure we agree on the
final
version of VIR_DIV_UP being committed.
One possible issue is that pre-patch [if value is int and size is a
power of 2], then INT_MAX / size * size is nicely truncated to INT_MAX &
~size, while post-patch, VIR_DIV_UP(INT_MAX, size) * size overflows.
That is, while we can tweak your patch to avoid overflow on the division
side, it doesn't affect overflow on the re-multiplication side, so
avoiding overflow is a bigger audit anyways.
That, and you've refactored it, so if we change our minds, it's now a
single line to change (rather than every caller of VIR_DIV_UP being
open-coded).
So let's push it as-is (but with an additional tweak to cover the new
divisions in 'virsh freecell --all' that were added in the meantime).
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library
http://libvirt.org