[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] Add VIR_DIV_UP to divide memory or storage request sizes with round up



On 01/28/2011 02:14 PM, Matthias Bolte wrote:
> Use it in all places where a memory or storage request size is converted
> to a larger granularity. This avoids requesting too small memory or storage
> sizes that could result from the truncation done by a simple division.

Much nicer than open-coding it everywhere.

>  141 files changed, 309 insertions(+), 326 deletions(-)

Wow - are we THAT guilty of copy-and-paste?  I hope you automated some
of these edits, rather than doing it all by hand (perl, or even
sed+xargs, can be pretty powerful).

> +++ b/src/internal.h
> @@ -231,4 +231,7 @@
>          }                                                               \
>      } while (0)
>  
> +/* 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?  That is, if value < max, but (value + size - 1) wraps
around, this could result in 0 (for unsigned types) or negative (for
signed types - in fact, overflow on signed types is technically
undefined behavior), where the old (unsafe) division would have resulted
in max/size.  Unfortunately, I don't see any way to do that without
evaluating value multiple times.  Any better ideas than:

# define VIR_DIV_UP(value, size) \
  (((value) + (size) - 1) < (value) \
   ? ((value) / (size)) + 1 \
   : ((value) + (size) - 1) / (size)

Also, is this the sort of thing where we should require that size be a
power of 2 and/or a compile-time constant (and thus that we can use <<
and >> instead of /)?  Then again, if size is a compile time constant
(which it looks like in all? of your cases), then the compiler should be
smart enough to optimize this (if not, you have a sorry compiler).

ACK to everything else, but let's make sure we agree on the final
version of VIR_DIV_UP being committed.

-- 
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]