[Qemu-devel] [PATCH v1 4/7] range: add some more functions

David Hildenbrand posted 7 patches 7 years ago
There is a newer version of this series
[Qemu-devel] [PATCH v1 4/7] range: add some more functions
Posted by David Hildenbrand 7 years ago
Add some more functions that will be used in memory-device context.

range_init(): Init using lower bound and size
range_valid(): Check if there would be an overflow when initializin
range_size(): Extract the size of a range
range_overlaps_range(): Check for overlaps of two ranges
range_contains_range(): Check if one range is contained in the other
range_starts_before_range(): Check if one range starts before another
range_ends_after_range(): Check if one range ends after another

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 include/qemu/range.h | 80 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 80 insertions(+)

diff --git a/include/qemu/range.h b/include/qemu/range.h
index 7e75f4e655..18e8acf22f 100644
--- a/include/qemu/range.h
+++ b/include/qemu/range.h
@@ -112,6 +112,86 @@ static inline uint64_t range_upb(Range *range)
     return range->upb;
 }
 
+/*
+ * Initialize @range to span the interval [@lob,@lob + @size - 1].
+ * @size may be 0.
+ */
+static inline void range_init(Range *range, uint64_t lob, uint64_t size)
+{
+    range->lob = lob;
+    range->upb = lob + size - 1;
+    range_invariant(range);
+}
+
+/*
+ * Check if the interval [@lob,@lob + @size - 1] would be valid or not
+ * (result in an overflow).
+ */
+static inline bool range_valid(uint64_t lob, uint64_t size)
+{
+    return lob + size >= lob;
+}
+
+/*
+ * Get the size of @range.
+ */
+static inline uint64_t range_size(const Range *range)
+{
+    return range->upb - range->lob + 1;
+}
+
+/*
+ * Check if @range1 overlaps with @range2. If one of the ranges is empty,
+ * the result is always "false".
+ */
+static inline bool range_overlaps_range(const Range *range1,
+                                        const Range *range2)
+{
+    if (range_is_empty(range1) || range_is_empty(range2)) {
+        return false;
+    }
+    return !(range2->upb < range1->lob || range1->upb < range2->lob);
+}
+
+/*
+ * Check if @range1 contains @range2. If one of the ranges is empty,
+ * the result is always "false".
+ */
+static inline bool range_contains_range(const Range *range1,
+                                        const Range *range2)
+{
+    if (range_is_empty(range1) || range_is_empty(range2)) {
+        return false;
+    }
+    return range1->lob <= range2->lob && range1->upb >= range2->upb;
+}
+
+/*
+ * Check if @range1 starts before @range2. If one of the ranges is empty,
+ * the result is alsways "false".
+ */
+static inline bool range_starts_before_range(const Range *range1,
+                                             const Range *range2)
+{
+    if (range_is_empty(range1) || range_is_empty(range2)) {
+        return false;
+    }
+    return range1->lob < range2->lob;
+}
+
+/*
+ * Check if @range1 ends after @range2. If one of the ranges is empty,
+ * the result is alsways "false".
+ */
+static inline bool range_ends_after_range(const Range *range1,
+                                          const Range *range2)
+{
+    if (range_is_empty(range1) || range_is_empty(range2)) {
+        return false;
+    }
+    return range1->upb > range2->upb;
+}
+
 /*
  * Extend @range to the smallest interval that includes @extend_by, too.
  */
-- 
2.17.1


Re: [Qemu-devel] [PATCH v1 4/7] range: add some more functions
Posted by Dr. David Alan Gilbert 7 years ago
* David Hildenbrand (david@redhat.com) wrote:
> Add some more functions that will be used in memory-device context.
> 
> range_init(): Init using lower bound and size
> range_valid(): Check if there would be an overflow when initializin
> range_size(): Extract the size of a range
> range_overlaps_range(): Check for overlaps of two ranges
> range_contains_range(): Check if one range is contained in the other
> range_starts_before_range(): Check if one range starts before another
> range_ends_after_range(): Check if one range ends after another
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  include/qemu/range.h | 80 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 80 insertions(+)
> 
> diff --git a/include/qemu/range.h b/include/qemu/range.h
> index 7e75f4e655..18e8acf22f 100644
> --- a/include/qemu/range.h
> +++ b/include/qemu/range.h
> @@ -112,6 +112,86 @@ static inline uint64_t range_upb(Range *range)
>      return range->upb;
>  }
>  
> +/*
> + * Initialize @range to span the interval [@lob,@lob + @size - 1].
> + * @size may be 0.
> + */
> +static inline void range_init(Range *range, uint64_t lob, uint64_t size)
> +{
> +    range->lob = lob;
> +    range->upb = lob + size - 1;
> +    range_invariant(range);
> +}
> +
> +/*
> + * Check if the interval [@lob,@lob + @size - 1] would be valid or not
> + * (result in an overflow).
> + */
> +static inline bool range_valid(uint64_t lob, uint64_t size)
> +{
> +    return lob + size >= lob;
> +}

That name confused me, I'd expected that to have taken a range and check
it for something (like a non-asserting version of the invariant).

> +/*
> + * Get the size of @range.
> + */
> +static inline uint64_t range_size(const Range *range)
> +{
> +    return range->upb - range->lob + 1;
> +}
> +
> +/*
> + * Check if @range1 overlaps with @range2. If one of the ranges is empty,
> + * the result is always "false".
> + */
> +static inline bool range_overlaps_range(const Range *range1,
> +                                        const Range *range2)
> +{
> +    if (range_is_empty(range1) || range_is_empty(range2)) {
> +        return false;
> +    }
> +    return !(range2->upb < range1->lob || range1->upb < range2->lob);
> +}
> +
> +/*
> + * Check if @range1 contains @range2. If one of the ranges is empty,
> + * the result is always "false".
> + */
> +static inline bool range_contains_range(const Range *range1,
> +                                        const Range *range2)
> +{
> +    if (range_is_empty(range1) || range_is_empty(range2)) {
> +        return false;
> +    }
> +    return range1->lob <= range2->lob && range1->upb >= range2->upb;
> +}
> +
> +/*
> + * Check if @range1 starts before @range2. If one of the ranges is empty,
> + * the result is alsways "false".
> + */
> +static inline bool range_starts_before_range(const Range *range1,
> +                                             const Range *range2)
> +{
> +    if (range_is_empty(range1) || range_is_empty(range2)) {
> +        return false;
> +    }
> +    return range1->lob < range2->lob;
> +}
> +
> +/*
> + * Check if @range1 ends after @range2. If one of the ranges is empty,
> + * the result is alsways "false".
> + */
> +static inline bool range_ends_after_range(const Range *range1,
> +                                          const Range *range2)
> +{
> +    if (range_is_empty(range1) || range_is_empty(range2)) {
> +        return false;
> +    }
> +    return range1->upb > range2->upb;
> +}
> +
>  /*
>   * Extend @range to the smallest interval that includes @extend_by, too.
>   */
> -- 
> 2.17.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v1 4/7] range: add some more functions
Posted by David Hildenbrand 7 years ago
On 11/10/2018 11:08, Dr. David Alan Gilbert wrote:
> * David Hildenbrand (david@redhat.com) wrote:
>> Add some more functions that will be used in memory-device context.
>>
>> range_init(): Init using lower bound and size
>> range_valid(): Check if there would be an overflow when initializin
>> range_size(): Extract the size of a range
>> range_overlaps_range(): Check for overlaps of two ranges
>> range_contains_range(): Check if one range is contained in the other
>> range_starts_before_range(): Check if one range starts before another
>> range_ends_after_range(): Check if one range ends after another
>>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>>  include/qemu/range.h | 80 ++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 80 insertions(+)
>>
>> diff --git a/include/qemu/range.h b/include/qemu/range.h
>> index 7e75f4e655..18e8acf22f 100644
>> --- a/include/qemu/range.h
>> +++ b/include/qemu/range.h
>> @@ -112,6 +112,86 @@ static inline uint64_t range_upb(Range *range)
>>      return range->upb;
>>  }
>>  
>> +/*
>> + * Initialize @range to span the interval [@lob,@lob + @size - 1].
>> + * @size may be 0.
>> + */
>> +static inline void range_init(Range *range, uint64_t lob, uint64_t size)
>> +{
>> +    range->lob = lob;
>> +    range->upb = lob + size - 1;
>> +    range_invariant(range);
>> +}
>> +
>> +/*
>> + * Check if the interval [@lob,@lob + @size - 1] would be valid or not
>> + * (result in an overflow).
>> + */
>> +static inline bool range_valid(uint64_t lob, uint64_t size)
>> +{
>> +    return lob + size >= lob;
>> +}
> 
> That name confused me, I'd expected that to have taken a range and check
> it for something (like a non-asserting version of the invariant).

Then we have to remove all the variant asserts from the initializer
functions (well, because then it is no longer an invariant then). Other
ideas?

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH v1 4/7] range: add some more functions
Posted by Dr. David Alan Gilbert 7 years ago
* David Hildenbrand (david@redhat.com) wrote:
> On 11/10/2018 11:08, Dr. David Alan Gilbert wrote:
> > * David Hildenbrand (david@redhat.com) wrote:
> >> Add some more functions that will be used in memory-device context.
> >>
> >> range_init(): Init using lower bound and size
> >> range_valid(): Check if there would be an overflow when initializin
> >> range_size(): Extract the size of a range
> >> range_overlaps_range(): Check for overlaps of two ranges
> >> range_contains_range(): Check if one range is contained in the other
> >> range_starts_before_range(): Check if one range starts before another
> >> range_ends_after_range(): Check if one range ends after another
> >>
> >> Signed-off-by: David Hildenbrand <david@redhat.com>
> >> ---
> >>  include/qemu/range.h | 80 ++++++++++++++++++++++++++++++++++++++++++++
> >>  1 file changed, 80 insertions(+)
> >>
> >> diff --git a/include/qemu/range.h b/include/qemu/range.h
> >> index 7e75f4e655..18e8acf22f 100644
> >> --- a/include/qemu/range.h
> >> +++ b/include/qemu/range.h
> >> @@ -112,6 +112,86 @@ static inline uint64_t range_upb(Range *range)
> >>      return range->upb;
> >>  }
> >>  
> >> +/*
> >> + * Initialize @range to span the interval [@lob,@lob + @size - 1].
> >> + * @size may be 0.
> >> + */
> >> +static inline void range_init(Range *range, uint64_t lob, uint64_t size)
> >> +{
> >> +    range->lob = lob;
> >> +    range->upb = lob + size - 1;
> >> +    range_invariant(range);
> >> +}
> >> +
> >> +/*
> >> + * Check if the interval [@lob,@lob + @size - 1] would be valid or not
> >> + * (result in an overflow).
> >> + */
> >> +static inline bool range_valid(uint64_t lob, uint64_t size)
> >> +{
> >> +    return lob + size >= lob;
> >> +}
> > 
> > That name confused me, I'd expected that to have taken a range and check
> > it for something (like a non-asserting version of the invariant).
> 
> Then we have to remove all the variant asserts from the initializer
> functions (well, because then it is no longer an invariant then). Other
> ideas?

My worry here is just the name 'range_valid'.

Dave

> -- 
> 
> Thanks,
> 
> David / dhildenb
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v1 4/7] range: add some more functions
Posted by David Hildenbrand 7 years ago
On 11/10/2018 11:21, Dr. David Alan Gilbert wrote:
> * David Hildenbrand (david@redhat.com) wrote:
>> On 11/10/2018 11:08, Dr. David Alan Gilbert wrote:
>>> * David Hildenbrand (david@redhat.com) wrote:
>>>> Add some more functions that will be used in memory-device context.
>>>>
>>>> range_init(): Init using lower bound and size
>>>> range_valid(): Check if there would be an overflow when initializin
>>>> range_size(): Extract the size of a range
>>>> range_overlaps_range(): Check for overlaps of two ranges
>>>> range_contains_range(): Check if one range is contained in the other
>>>> range_starts_before_range(): Check if one range starts before another
>>>> range_ends_after_range(): Check if one range ends after another
>>>>
>>>> Signed-off-by: David Hildenbrand <david@redhat.com>
>>>> ---
>>>>  include/qemu/range.h | 80 ++++++++++++++++++++++++++++++++++++++++++++
>>>>  1 file changed, 80 insertions(+)
>>>>
>>>> diff --git a/include/qemu/range.h b/include/qemu/range.h
>>>> index 7e75f4e655..18e8acf22f 100644
>>>> --- a/include/qemu/range.h
>>>> +++ b/include/qemu/range.h
>>>> @@ -112,6 +112,86 @@ static inline uint64_t range_upb(Range *range)
>>>>      return range->upb;
>>>>  }
>>>>  
>>>> +/*
>>>> + * Initialize @range to span the interval [@lob,@lob + @size - 1].
>>>> + * @size may be 0.
>>>> + */
>>>> +static inline void range_init(Range *range, uint64_t lob, uint64_t size)
>>>> +{
>>>> +    range->lob = lob;
>>>> +    range->upb = lob + size - 1;
>>>> +    range_invariant(range);
>>>> +}
>>>> +
>>>> +/*
>>>> + * Check if the interval [@lob,@lob + @size - 1] would be valid or not
>>>> + * (result in an overflow).
>>>> + */
>>>> +static inline bool range_valid(uint64_t lob, uint64_t size)
>>>> +{
>>>> +    return lob + size >= lob;
>>>> +}
>>>
>>> That name confused me, I'd expected that to have taken a range and check
>>> it for something (like a non-asserting version of the invariant).
>>
>> Then we have to remove all the variant asserts from the initializer
>> functions (well, because then it is no longer an invariant then). Other
>> ideas?
> 
> My worry here is just the name 'range_valid'.
> 

hmm "range_would_overflow()" ?

> Dave

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH v1 4/7] range: add some more functions
Posted by Dr. David Alan Gilbert 7 years ago
* David Hildenbrand (david@redhat.com) wrote:
> On 11/10/2018 11:21, Dr. David Alan Gilbert wrote:
> > * David Hildenbrand (david@redhat.com) wrote:
> >> On 11/10/2018 11:08, Dr. David Alan Gilbert wrote:
> >>> * David Hildenbrand (david@redhat.com) wrote:
> >>>> Add some more functions that will be used in memory-device context.
> >>>>
> >>>> range_init(): Init using lower bound and size
> >>>> range_valid(): Check if there would be an overflow when initializin
> >>>> range_size(): Extract the size of a range
> >>>> range_overlaps_range(): Check for overlaps of two ranges
> >>>> range_contains_range(): Check if one range is contained in the other
> >>>> range_starts_before_range(): Check if one range starts before another
> >>>> range_ends_after_range(): Check if one range ends after another
> >>>>
> >>>> Signed-off-by: David Hildenbrand <david@redhat.com>
> >>>> ---
> >>>>  include/qemu/range.h | 80 ++++++++++++++++++++++++++++++++++++++++++++
> >>>>  1 file changed, 80 insertions(+)
> >>>>
> >>>> diff --git a/include/qemu/range.h b/include/qemu/range.h
> >>>> index 7e75f4e655..18e8acf22f 100644
> >>>> --- a/include/qemu/range.h
> >>>> +++ b/include/qemu/range.h
> >>>> @@ -112,6 +112,86 @@ static inline uint64_t range_upb(Range *range)
> >>>>      return range->upb;
> >>>>  }
> >>>>  
> >>>> +/*
> >>>> + * Initialize @range to span the interval [@lob,@lob + @size - 1].
> >>>> + * @size may be 0.
> >>>> + */
> >>>> +static inline void range_init(Range *range, uint64_t lob, uint64_t size)
> >>>> +{
> >>>> +    range->lob = lob;
> >>>> +    range->upb = lob + size - 1;
> >>>> +    range_invariant(range);
> >>>> +}
> >>>> +
> >>>> +/*
> >>>> + * Check if the interval [@lob,@lob + @size - 1] would be valid or not
> >>>> + * (result in an overflow).
> >>>> + */
> >>>> +static inline bool range_valid(uint64_t lob, uint64_t size)
> >>>> +{
> >>>> +    return lob + size >= lob;
> >>>> +}
> >>>
> >>> That name confused me, I'd expected that to have taken a range and check
> >>> it for something (like a non-asserting version of the invariant).
> >>
> >> Then we have to remove all the variant asserts from the initializer
> >> functions (well, because then it is no longer an invariant then). Other
> >> ideas?
> > 
> > My worry here is just the name 'range_valid'.
> > 
> 
> hmm "range_would_overflow()" ?

Yes, a bit long but OK.

But another observation; in the following patch, you're tending to do:

  if (!range_valid(...))
     moan


  range_init(...)

would it make more sense to change range_init so it was:

static inline bool range_init(Range *range, uint64_t lob, uint64_t size)
{
    range->lob = lob;
    range->upb = lob + size - 1;
    return ob + size >= lob;
}


and then in the places you use it, you could do:

  if (!range_init(...)
    moan


Dave

> > Dave
> 
> -- 
> 
> Thanks,
> 
> David / dhildenb
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v1 4/7] range: add some more functions
Posted by David Hildenbrand 7 years ago
On 11/10/2018 12:27, Dr. David Alan Gilbert wrote:
> * David Hildenbrand (david@redhat.com) wrote:
>> On 11/10/2018 11:21, Dr. David Alan Gilbert wrote:
>>> * David Hildenbrand (david@redhat.com) wrote:
>>>> On 11/10/2018 11:08, Dr. David Alan Gilbert wrote:
>>>>> * David Hildenbrand (david@redhat.com) wrote:
>>>>>> Add some more functions that will be used in memory-device context.
>>>>>>
>>>>>> range_init(): Init using lower bound and size
>>>>>> range_valid(): Check if there would be an overflow when initializin
>>>>>> range_size(): Extract the size of a range
>>>>>> range_overlaps_range(): Check for overlaps of two ranges
>>>>>> range_contains_range(): Check if one range is contained in the other
>>>>>> range_starts_before_range(): Check if one range starts before another
>>>>>> range_ends_after_range(): Check if one range ends after another
>>>>>>
>>>>>> Signed-off-by: David Hildenbrand <david@redhat.com>
>>>>>> ---
>>>>>>  include/qemu/range.h | 80 ++++++++++++++++++++++++++++++++++++++++++++
>>>>>>  1 file changed, 80 insertions(+)
>>>>>>
>>>>>> diff --git a/include/qemu/range.h b/include/qemu/range.h
>>>>>> index 7e75f4e655..18e8acf22f 100644
>>>>>> --- a/include/qemu/range.h
>>>>>> +++ b/include/qemu/range.h
>>>>>> @@ -112,6 +112,86 @@ static inline uint64_t range_upb(Range *range)
>>>>>>      return range->upb;
>>>>>>  }
>>>>>>  
>>>>>> +/*
>>>>>> + * Initialize @range to span the interval [@lob,@lob + @size - 1].
>>>>>> + * @size may be 0.
>>>>>> + */
>>>>>> +static inline void range_init(Range *range, uint64_t lob, uint64_t size)
>>>>>> +{
>>>>>> +    range->lob = lob;
>>>>>> +    range->upb = lob + size - 1;
>>>>>> +    range_invariant(range);
>>>>>> +}
>>>>>> +
>>>>>> +/*
>>>>>> + * Check if the interval [@lob,@lob + @size - 1] would be valid or not
>>>>>> + * (result in an overflow).
>>>>>> + */
>>>>>> +static inline bool range_valid(uint64_t lob, uint64_t size)
>>>>>> +{
>>>>>> +    return lob + size >= lob;
>>>>>> +}
>>>>>
>>>>> That name confused me, I'd expected that to have taken a range and check
>>>>> it for something (like a non-asserting version of the invariant).
>>>>
>>>> Then we have to remove all the variant asserts from the initializer
>>>> functions (well, because then it is no longer an invariant then). Other
>>>> ideas?
>>>
>>> My worry here is just the name 'range_valid'.
>>>
>>
>> hmm "range_would_overflow()" ?
> 
> Yes, a bit long but OK.
> 
> But another observation; in the following patch, you're tending to do:
> 
>   if (!range_valid(...))
>      moan
> 
> 
>   range_init(...)
> 
> would it make more sense to change range_init so it was:
> 
> static inline bool range_init(Range *range, uint64_t lob, uint64_t size)
> {
>     range->lob = lob;
>     range->upb = lob + size - 1;
>     return ob + size >= lob;
> }
> 
> 
> and then in the places you use it, you could do:
> 
>   if (!range_init(...)
>     moan
> 

Yes, that makes sense.


-- 

Thanks,

David / dhildenb