From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id DB58B430409 for ; Thu, 3 Sep 2020 10:19:04 +0300 (MSK) References: <20200901162751.23037-1-i.kosarev@tarantool.org> From: Aleksandr Lyapunov Message-ID: Date: Thu, 3 Sep 2020 10:19:03 +0300 MIME-Version: 1.0 In-Reply-To: <20200901162751.23037-1-i.kosarev@tarantool.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US Subject: Re: [Tarantool-patches] [PATCH] rtree: add comments on ignored rtree_search() return value List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Ilya Kosarev Cc: tarantool-patches@dev.tarantool.org Hi, thanks for the patch, LGTM On 01.09.2020 19:27, Ilya Kosarev wrote: > rtree_search() has return value and it is ignored in some cases. > Although it is totally fine it seems to be reasonable to comment those > cases as far as such usage might be questionable. > > Closes #2052 > --- > Branch: https://github.com/tarantool/tarantool/tree/i.kosarev/gh-2052-rtree_search()-result-ignored > Issue: https://github.com/tarantool/tarantool/issues/2052 > > src/box/memtx_rtree.c | 7 +++++++ > test/unit/rtree_iterator.cc | 7 +++++++ > 2 files changed, 14 insertions(+) > > diff --git a/src/box/memtx_rtree.c b/src/box/memtx_rtree.c > index 612fcb2a9f..6b8b0a2e17 100644 > --- a/src/box/memtx_rtree.c > +++ b/src/box/memtx_rtree.c > @@ -322,6 +322,13 @@ memtx_rtree_index_create_iterator(struct index *base, enum iterator_type type, > it->base.next = index_rtree_iterator_next; > it->base.free = index_rtree_iterator_free; > rtree_iterator_init(&it->impl); > + /* > + * We don't care if rtree_search() does or does not find anything > + * as far as we are fine anyway: iterator is initialized correctly > + * and the case where nothing is found and rtree_iterator_next() > + * returns NULL should be processed in the correct way on the > + * caller's side. > + */ > rtree_search(&index->tree, &rect, op, &it->impl); > return (struct iterator *)it; > } > diff --git a/test/unit/rtree_iterator.cc b/test/unit/rtree_iterator.cc > index b3c8695e9d..641b659177 100644 > --- a/test/unit/rtree_iterator.cc > +++ b/test/unit/rtree_iterator.cc > @@ -274,6 +274,13 @@ iterator_invalidate_check() > rtree_insert(&tree, &rect, record_t(i+1)); > } > rtree_set2d(&rect, 0, 0, test_size, test_size); > + /* > + * We don't care if rtree_search() does or does not find > + * anything as far as we are fine anyway: iterator is > + * initialized correctly and the case where nothing is found > + * and rtree_iterator_next() returns NULL will be processed > + * correctly as fail case in check #19. > + */ > rtree_search(&tree, &rect, SOP_BELONGS, &iterators[0]); > if (!rtree_iterator_next(&iterators[0])) { > fail("Integrity check failed (19)", "false");