Commit 0be4d2fe62893b8450978287a6814284b0314d5e
1 parent
f8b548c8
Refactor `NNTreeImpl::find`: update key parameter to `const&`, replace `getKey` …
…with subscript operator, simplify logic, and improve error checks.
Showing
2 changed files
with
21 additions
and
26 deletions
libqpdf/NNTree.cc
| @@ -674,7 +674,7 @@ NNTreeImpl::repair() | @@ -674,7 +674,7 @@ NNTreeImpl::repair() | ||
| 674 | } | 674 | } |
| 675 | 675 | ||
| 676 | NNTreeImpl::iterator | 676 | NNTreeImpl::iterator |
| 677 | -NNTreeImpl::find(QPDFObjectHandle key, bool return_prev_if_not_found) | 677 | +NNTreeImpl::find(QPDFObjectHandle const& key, bool return_prev_if_not_found) |
| 678 | { | 678 | { |
| 679 | try { | 679 | try { |
| 680 | return findInternal(key, return_prev_if_not_found); | 680 | return findInternal(key, return_prev_if_not_found); |
| @@ -693,23 +693,19 @@ NNTreeImpl::iterator | @@ -693,23 +693,19 @@ NNTreeImpl::iterator | ||
| 693 | NNTreeImpl::findInternal(QPDFObjectHandle const& key, bool return_prev_if_not_found) | 693 | NNTreeImpl::findInternal(QPDFObjectHandle const& key, bool return_prev_if_not_found) |
| 694 | { | 694 | { |
| 695 | auto first_item = begin(); | 695 | auto first_item = begin(); |
| 696 | - auto last_item = end(); | ||
| 697 | - if (first_item == end()) { | 696 | + if (!first_item.valid()) { |
| 698 | return end(); | 697 | return end(); |
| 699 | } | 698 | } |
| 700 | - if (first_item.valid()) { | ||
| 701 | - if (!keyValid(first_item->first)) { | ||
| 702 | - error(oh, "encountered invalid key in find"); | ||
| 703 | - } | ||
| 704 | - if (!value_valid(first_item->second)) { | ||
| 705 | - error(oh, "encountered invalid value in find"); | ||
| 706 | - } | ||
| 707 | - if (compareKeys(key, first_item->first) < 0) { | ||
| 708 | - // Before the first key | ||
| 709 | - return end(); | ||
| 710 | - } | 699 | + if (!keyValid(first_item->first)) { |
| 700 | + error(oh, "encountered invalid key in find"); | ||
| 701 | + } | ||
| 702 | + if (!value_valid(first_item->second)) { | ||
| 703 | + error(oh, "encountered invalid value in find"); | ||
| 704 | + } | ||
| 705 | + if (compareKeys(key, first_item->first) < 0) { | ||
| 706 | + // Before the first key | ||
| 707 | + return end(); | ||
| 711 | } | 708 | } |
| 712 | - qpdf_assert_debug(!last_item.valid()); | ||
| 713 | 709 | ||
| 714 | QPDFObjGen::set seen; | 710 | QPDFObjGen::set seen; |
| 715 | auto node = oh; | 711 | auto node = oh; |
| @@ -720,7 +716,7 @@ NNTreeImpl::findInternal(QPDFObjectHandle const& key, bool return_prev_if_not_fo | @@ -720,7 +716,7 @@ NNTreeImpl::findInternal(QPDFObjectHandle const& key, bool return_prev_if_not_fo | ||
| 720 | error(node, "loop detected in find"); | 716 | error(node, "loop detected in find"); |
| 721 | } | 717 | } |
| 722 | 718 | ||
| 723 | - Array items = node.getKey(itemsKey()); | 719 | + Array items = node[itemsKey()]; |
| 724 | size_t nitems = items.size(); | 720 | size_t nitems = items.size(); |
| 725 | if (nitems > 1) { | 721 | if (nitems > 1) { |
| 726 | int idx = binarySearch( | 722 | int idx = binarySearch( |
| @@ -737,18 +733,17 @@ NNTreeImpl::findInternal(QPDFObjectHandle const& key, bool return_prev_if_not_fo | @@ -737,18 +733,17 @@ NNTreeImpl::findInternal(QPDFObjectHandle const& key, bool return_prev_if_not_fo | ||
| 737 | return result; | 733 | return result; |
| 738 | } | 734 | } |
| 739 | 735 | ||
| 740 | - Array kids = node.getKey("/Kids"); | 736 | + Array kids = node["/Kids"]; |
| 741 | size_t nkids = kids.size(); | 737 | size_t nkids = kids.size(); |
| 742 | - if (nkids > 0) { | ||
| 743 | - int idx = binarySearch(key, kids, nkids, true, &NNTreeImpl::compareKeyKid); | ||
| 744 | - if (idx == -1) { | ||
| 745 | - error(node, "unexpected -1 from binary search of kids; limits may by wrong"); | ||
| 746 | - } | ||
| 747 | - result.addPathElement(node, idx); | ||
| 748 | - node = kids[idx]; | ||
| 749 | - } else { | 738 | + if (nkids == 0) { |
| 750 | error(node, "bad node during find"); | 739 | error(node, "bad node during find"); |
| 751 | } | 740 | } |
| 741 | + int idx = binarySearch(key, kids, nkids, true, &NNTreeImpl::compareKeyKid); | ||
| 742 | + if (idx == -1) { | ||
| 743 | + error(node, "unexpected -1 from binary search of kids; limits may by wrong"); | ||
| 744 | + } | ||
| 745 | + result.addPathElement(node, idx); | ||
| 746 | + node = kids[idx]; | ||
| 752 | } | 747 | } |
| 753 | } | 748 | } |
| 754 | 749 |
libqpdf/qpdf/NNTree.hh
| @@ -147,7 +147,7 @@ class NNTreeImpl final | @@ -147,7 +147,7 @@ class NNTreeImpl final | ||
| 147 | return {*this}; | 147 | return {*this}; |
| 148 | } | 148 | } |
| 149 | iterator last(); | 149 | iterator last(); |
| 150 | - iterator find(QPDFObjectHandle key, bool return_prev_if_not_found = false); | 150 | + iterator find(QPDFObjectHandle const& key, bool return_prev_if_not_found = false); |
| 151 | iterator insertFirst(QPDFObjectHandle const& key, QPDFObjectHandle const& value); | 151 | iterator insertFirst(QPDFObjectHandle const& key, QPDFObjectHandle const& value); |
| 152 | iterator insert(QPDFObjectHandle const& key, QPDFObjectHandle const& value); | 152 | iterator insert(QPDFObjectHandle const& key, QPDFObjectHandle const& value); |
| 153 | bool remove(QPDFObjectHandle const& key, QPDFObjectHandle* value = nullptr); | 153 | bool remove(QPDFObjectHandle const& key, QPDFObjectHandle* value = nullptr); |