From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41434) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gAYBp-0000Dg-C9 for qemu-devel@nongnu.org; Thu, 11 Oct 2018 06:27:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gAYBm-0005Uo-4C for qemu-devel@nongnu.org; Thu, 11 Oct 2018 06:27:33 -0400 Received: from mx1.redhat.com ([209.132.183.28]:48468) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gAYBl-0005OJ-Nj for qemu-devel@nongnu.org; Thu, 11 Oct 2018 06:27:29 -0400 Date: Thu, 11 Oct 2018 11:27:05 +0100 From: "Dr. David Alan Gilbert" Message-ID: <20181011102705.GF2483@work-vm> References: <20181009205652.10605-1-david@redhat.com> <20181009205652.10605-5-david@redhat.com> <20181011090821.GC2483@work-vm> <20181011092132.GD2483@work-vm> <733ce1f0-c50f-b856-500b-e995d999605e@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <733ce1f0-c50f-b856-500b-e995d999605e@redhat.com> Subject: Re: [Qemu-devel] [PATCH v1 4/7] range: add some more functions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Hildenbrand Cc: qemu-devel@nongnu.org, "Michael S . Tsirkin" , Igor Mammedov , Markus Armbruster , Michael Roth , David Gibson , Eduardo Habkost * 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 > >>>> --- > >>>> 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