* [Tarantool-patches] [PATCH] test: fix local array access in heap_iterator unit test @ 2020-12-03 14:38 Kirill Yukhin 2020-12-04 6:33 ` Aleksandr Lyapunov 0 siblings, 1 reply; 4+ messages in thread From: Kirill Yukhin @ 2020-12-03 14:38 UTC (permalink / raw) To: alyapunov, avtikhon; +Cc: tarantool-patches 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) fail("from iterator returned incorrect value", "val < 1 || val > 5"); - if (used_key[val]) + if (used_key[val - 1]) fail("from iterator some value returned twice", "used[val]"); - used_key[val] = 1; + used_key[val - 1] = 1; } bool f = true; for (uint32_t i = 1; i < 5; ++i) - f = used_key[i] && f; + f = used_key[i - 1] && f; if (!f) fail("some node was skipped", "!f"); -- 2.11.0 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Tarantool-patches] [PATCH] test: fix local array access in heap_iterator unit test 2020-12-03 14:38 [Tarantool-patches] [PATCH] test: fix local array access in heap_iterator unit test Kirill Yukhin @ 2020-12-04 6:33 ` Aleksandr Lyapunov 2020-12-04 8:54 ` Kirill Yukhin 0 siblings, 1 reply; 4+ messages in thread From: Aleksandr Lyapunov @ 2020-12-04 6:33 UTC (permalink / raw) To: Kirill Yukhin, avtikhon; +Cc: tarantool-patches 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. > fail("from iterator returned incorrect value", > "val < 1 || val > 5"); > - if (used_key[val]) > + if (used_key[val - 1]) > fail("from iterator some value returned twice", > "used[val]"); > - used_key[val] = 1; > + used_key[val - 1] = 1; > } > > bool f = true; > for (uint32_t i = 1; i < 5; ++i) > - f = used_key[i] && f; > + f = used_key[i - 1] && f; > if (!f) > fail("some node was skipped", "!f"); > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Tarantool-patches] [PATCH] test: fix local array access in heap_iterator unit test 2020-12-04 6:33 ` Aleksandr Lyapunov @ 2020-12-04 8:54 ` Kirill Yukhin 2020-12-07 12:24 ` Aleksandr Lyapunov 0 siblings, 1 reply; 4+ messages in thread From: Kirill Yukhin @ 2020-12-04 8:54 UTC (permalink / raw) To: Aleksandr Lyapunov; +Cc: tarantool-patches 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 <kyukhin@tarantool.org> 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"); ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Tarantool-patches] [PATCH] test: fix local array access in heap_iterator unit test 2020-12-04 8:54 ` Kirill Yukhin @ 2020-12-07 12:24 ` Aleksandr Lyapunov 0 siblings, 0 replies; 4+ messages in thread From: Aleksandr Lyapunov @ 2020-12-07 12:24 UTC (permalink / raw) To: Kirill Yukhin; +Cc: tarantool-patches Hi, thanks for the path! LGTM. On 04.12.2020 11:54, Kirill Yukhin wrote: > 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 <kyukhin@tarantool.org> > 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"); ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-12-07 12:25 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-12-03 14:38 [Tarantool-patches] [PATCH] test: fix local array access in heap_iterator unit test Kirill Yukhin 2020-12-04 6:33 ` Aleksandr Lyapunov 2020-12-04 8:54 ` Kirill Yukhin 2020-12-07 12:24 ` Aleksandr Lyapunov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox