[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 01/10] hbitmap: introduce HBITMAP_MAX_ORIG_SIZE
From: |
Vladimir Sementsov-Ogievskiy |
Subject: |
Re: [PATCH 01/10] hbitmap: introduce HBITMAP_MAX_ORIG_SIZE |
Date: |
Wed, 9 Oct 2019 16:04:23 +0000 |
09.10.2019 18:34, Eric Blake wrote:
> On 9/30/19 10:14 AM, Vladimir Sementsov-Ogievskiy wrote:
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
>
> A bit light on the commit message for explaining why.
If use just INT64_MAX, the comment below may be move here, accompanied by
your note about disk size limit.
>
>> ---
>> include/qemu/hbitmap.h | 7 +++++++
>> util/hbitmap.c | 2 ++
>> 2 files changed, 9 insertions(+)
>>
>> diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
>> index 1bf944ca3d..82317c5364 100644
>> --- a/include/qemu/hbitmap.h
>> +++ b/include/qemu/hbitmap.h
>> @@ -33,6 +33,13 @@ typedef struct HBitmapIter HBitmapIter;
>> */
>> #define HBITMAP_LEVELS ((HBITMAP_LOG_MAX_SIZE / BITS_PER_LEVEL) +
>> 1)
>> +/*
>> + * We have APIs which returns signed int64_t, to be able to return error.
>> + * Therefore we can't handle bitmaps with absolute size larger than
>> + * (INT64_MAX+1). Still, keep it INT64_MAX to be a bit safer.
>> + */
>> +#define HBITMAP_MAX_ORIG_SIZE INT64_MAX
>
> That, and bitmaps represent disk images, but disk images can't exceed
> INT64_MAX bytes (thanks to off_t being signed). But does introducing a new
> constant really help?
>
>> +
>> struct HBitmapIter {
>> const HBitmap *hb;
>> diff --git a/util/hbitmap.c b/util/hbitmap.c
>> index 757d39e360..df192234e3 100644
>> --- a/util/hbitmap.c
>> +++ b/util/hbitmap.c
>> @@ -708,6 +708,7 @@ HBitmap *hbitmap_alloc(uint64_t size, int granularity)
>> HBitmap *hb = g_new0(struct HBitmap, 1);
>> unsigned i;
>> + assert(size <= HBITMAP_MAX_ORIG_SIZE);
>
> or can we just inline INT64_MAX here?
OK for me too.
>
>> hb->orig_size = size;
>> assert(granularity >= 0 && granularity < 64);
>> @@ -738,6 +739,7 @@ void hbitmap_truncate(HBitmap *hb, uint64_t size)
>> uint64_t num_elements = size;
>> uint64_t old;
>> + assert(size <= HBITMAP_MAX_ORIG_SIZE);
>> hb->orig_size = size;
>> /* Size comes in as logical elements, adjust for granularity. */
>>
>
--
Best regards,
Vladimir