[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@xxxxxxxxxx +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list