Skip to content
This repository was archived by the owner on Mar 22, 2023. It is now read-only.

radix_tree: fix upper_bound implementation#906

Merged
szyrom merged 2 commits intopmem:masterfrom
igchor:fix_radix_upper_bound
Sep 30, 2020
Merged

radix_tree: fix upper_bound implementation#906
szyrom merged 2 commits intopmem:masterfrom
igchor:fix_radix_upper_bound

Conversation

@igchor
Copy link
Copy Markdown
Contributor

@igchor igchor commented Sep 30, 2020

If a key was not present in a radix tree and a slot variable was
set to &root, next_leaf function lead to accessing uninitialzied memory
(we tried to increment slot, as if it was a pointer to an element in array).


This change is Reviewable

@codecov
Copy link
Copy Markdown

codecov Bot commented Sep 30, 2020

Codecov Report

Merging #906 into master will increase coverage by 0.06%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #906      +/-   ##
==========================================
+ Coverage   95.95%   96.02%   +0.06%     
==========================================
  Files          48       48              
  Lines        6083     6085       +2     
==========================================
+ Hits         5837     5843       +6     
+ Misses        246      242       -4     
Flag Coverage Δ
#tests_clang_debug_cpp17 95.95% <100.00%> (+0.10%) ⬆️
#tests_gcc_debug 92.41% <100.00%> (+0.10%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
include/libpmemobj++/experimental/radix_tree.hpp 99.45% <100.00%> (+<0.01%) ⬆️
...ude/libpmemobj++/container/concurrent_hash_map.hpp 93.96% <0.00%> (+0.10%) ⬆️
...j++/container/detail/concurrent_skip_list_impl.hpp 98.12% <0.00%> (+0.13%) ⬆️
include/libpmemobj++/mutex.hpp 86.20% <0.00%> (+6.89%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 40b23f2...cb2091f. Read the comment docs.

Copy link
Copy Markdown

@szyrom szyrom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 2 of 2 files at r1.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

Copy link
Copy Markdown
Member

@lukaszstolarczuk lukaszstolarczuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 2 of 2 files at r1.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

If a key was not present in a radix tree and a slot variable was
set to &root, next_leaf function lead to accessing uninitialzied memory
(we tried to increment slot, as if it was a pointer to an element in array).
In internal_bound we actually rely on leaf node to be the leftmost one.
@igchor igchor force-pushed the fix_radix_upper_bound branch from 63b0daf to cb2091f Compare September 30, 2020 10:38
Copy link
Copy Markdown

@szyrom szyrom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 2 of 2 files at r2.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@igchor igchor marked this pull request as ready for review September 30, 2020 10:56
Copy link
Copy Markdown
Member

@lukaszstolarczuk lukaszstolarczuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 2 of 2 files at r2.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@szyrom szyrom merged commit 54aa36e into pmem:master Sep 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants