[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v1 4/7] range: add some more functions
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [PATCH v1 4/7] range: add some more functions |
Date: |
Thu, 11 Oct 2018 11:27:05 +0100 |
User-agent: |
Mutt/1.10.1 (2018-07-13) |
* David Hildenbrand (address@hidden) wrote:
> On 11/10/2018 11:21, Dr. David Alan Gilbert wrote:
> > * David Hildenbrand (address@hidden) wrote:
> >> On 11/10/2018 11:08, Dr. David Alan Gilbert wrote:
> >>> * David Hildenbrand (address@hidden) 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 <address@hidden>
> >>>> ---
> >>>> 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 address@hidden,@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 address@hidden,@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 / address@hidden / Manchester, UK
[Qemu-devel] [PATCH v1 7/7] memory-device: rewrite address assignment using ranges, David Hildenbrand, 2018/10/09
[Qemu-devel] [PATCH v1 6/7] memory-device: avoid overflows on very huge devices, David Hildenbrand, 2018/10/09
[Qemu-devel] [PATCH v1 5/7] memory-device: use QEMU_IS_ALIGNED, David Hildenbrand, 2018/10/09