Re: [Tarantool-patches] [PATCH] rtree: add comments on ignored rtree_search() return value

[Aleksandr Lyapunov <alyapunov@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");