From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp39.i.mail.ru (smtp39.i.mail.ru [94.100.177.99]) (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 A38E945C305 for ; Fri, 4 Dec 2020 11:54:26 +0300 (MSK) Date: Fri, 4 Dec 2020 08:54:22 +0000 From: Kirill Yukhin Message-ID: <20201204085422.36tzl24q2czeipte@tarantool.org> References: <86d41c348ae271f5a8a8dec6fdd4ccc045a0408d.1607006062.git.kyukhin@tarantool.org> <7bae6a08-1931-3616-4429-4685ea18737a@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <7bae6a08-1931-3616-4429-4685ea18737a@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH] test: fix local array access in heap_iterator unit test List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Aleksandr Lyapunov Cc: tarantool-patches@dev.tarantool.org Hello, On 04 Dec 09:33, Aleksandr Lyapunov wrote: > Hello! Thanks for the patch, see my comment below: > > On 03.12.2020 17:38, Kirill Yukhin wrote: > > Index variable run from 1 to 5 and was used to index > > array of size 4. Use iv - 1 instead. > > > > Discovered by Coverity. > > --- > > > > Issue: N/A > > Branch: https://github.com/tarantool/tarantool/tree/kyukhin/fix-heap_iterator > > CI: https://gitlab.com/tarantool/tarantool/-/pipelines/224811793 > > > > test/unit/heap_iterator.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/test/unit/heap_iterator.c b/test/unit/heap_iterator.c > > index 4fde39d..3fcefff 100644 > > --- a/test/unit/heap_iterator.c > > +++ b/test/unit/heap_iterator.c > > @@ -111,15 +111,15 @@ test_iterator_small() > > if (val < 1 || val > 5) > Accessing below by [val - 1] is ok, but the main problem is that this check > is wrong. > If the heap works correctly - 'val' must be in range [1..4]. > I think we should fix to "val < 1 || val > 4". > After that both variants of accessing array by [val] and [val - 1] would be > ok. I agree, thanks. Updated patch in the bottom. Branch force-pushed. I've accidentally committed this version to master. So, this is iterative. -- Regards, Kirill Yukhin Author: Kirill Yukhin Date: Fri Dec 4 11:50:31 2020 +0300 test: fixup unit/heap_iterator Accidentally committed [1] which was under review. This is proper fix. [1] - 86d41c348ae271f5a8a8dec6fdd4ccc045a0408d diff --git a/test/unit/heap_iterator.c b/test/unit/heap_iterator.c index 3fcefff..46723b3 100644 --- a/test/unit/heap_iterator.c +++ b/test/unit/heap_iterator.c @@ -98,7 +98,7 @@ test_iterator_small() } struct heap_iterator it; - bool used_key[5]; + bool used_key[4]; memset((void *)used_key, 0, sizeof(used_key)); test_heap_iterator_init(&heap, &it); @@ -108,9 +108,9 @@ test_iterator_small() if (value == NULL) fail("NULL returned from iterator", "value == NULL"); uint32_t val = value->val1; - if (val < 1 || val > 5) + if (val < 1 || val > 4) fail("from iterator returned incorrect value", - "val < 1 || val > 5"); + "val < 1 || val > 4"); if (used_key[val - 1]) fail("from iterator some value returned twice", "used[val]"); @@ -118,8 +118,8 @@ test_iterator_small() } bool f = true; - for (uint32_t i = 1; i < 5; ++i) - f = used_key[i - 1] && f; + for (uint32_t i = 0; i < 4; ++i) + f = used_key[i] && f; if (!f) fail("some node was skipped", "!f");