Commit 0d8b0882eaba9288501a7f1d96474b542f714199
1 parent
baa77f54
Refactor `NNTree::deepen`: introduce lambda for failure handling, simplify loop …
…logic, and replace redundant code with subscript operators.
Showing
2 changed files
with
23 additions
and
39 deletions
libqpdf/NNTree.cc
| @@ -592,74 +592,64 @@ NNTreeIterator::deepen(QPDFObjectHandle a_node, bool first, bool allow_empty) | @@ -592,74 +592,64 @@ NNTreeIterator::deepen(QPDFObjectHandle a_node, bool first, bool allow_empty) | ||
| 592 | // items. If we succeed, return true; otherwise return false and leave path alone. | 592 | // items. If we succeed, return true; otherwise return false and leave path alone. |
| 593 | 593 | ||
| 594 | auto opath = path; | 594 | auto opath = path; |
| 595 | - bool failed = false; | 595 | + |
| 596 | + auto fail = [this, &opath](QPDFObjectHandle const& failed_node, std::string const& msg) { | ||
| 597 | + impl.warn(failed_node, msg); | ||
| 598 | + path = opath; | ||
| 599 | + return false; | ||
| 600 | + }; | ||
| 596 | 601 | ||
| 597 | QPDFObjGen::set seen; | 602 | QPDFObjGen::set seen; |
| 598 | for (auto const& i: path) { | 603 | for (auto const& i: path) { |
| 599 | seen.add(i.node); | 604 | seen.add(i.node); |
| 600 | } | 605 | } |
| 601 | - while (!failed) { | 606 | + while (true) { |
| 602 | if (!seen.add(a_node)) { | 607 | if (!seen.add(a_node)) { |
| 603 | - QTC::TC("qpdf", "NNTree deepen: loop"); | ||
| 604 | - impl.warn(a_node, "loop detected while traversing name/number tree"); | ||
| 605 | - failed = true; | ||
| 606 | - break; | 608 | + return fail(a_node, "loop detected while traversing name/number tree"); |
| 607 | } | 609 | } |
| 608 | 610 | ||
| 609 | if (!a_node.isDictionary()) { | 611 | if (!a_node.isDictionary()) { |
| 610 | - QTC::TC("qpdf", "NNTree node is not a dictionary"); | ||
| 611 | - impl.warn(a_node, "non-dictionary node while traversing name/number tree"); | ||
| 612 | - failed = true; | ||
| 613 | - break; | 612 | + return fail(a_node, "non-dictionary node while traversing name/number tree"); |
| 614 | } | 613 | } |
| 615 | 614 | ||
| 616 | - auto kids = a_node.getKey("/Kids"); | ||
| 617 | - int nkids = kids.isArray() ? kids.getArrayNItems() : 0; | ||
| 618 | auto items = a_node.getKey(impl.details.itemsKey()); | 615 | auto items = a_node.getKey(impl.details.itemsKey()); |
| 619 | - int nitems = items.isArray() ? items.getArrayNItems() : 0; | ||
| 620 | - if (nitems > 0) { | 616 | + int nitems = static_cast<int>(items.size()); |
| 617 | + if (nitems > 1) { | ||
| 621 | setItemNumber(a_node, first ? 0 : nitems - 2); | 618 | setItemNumber(a_node, first ? 0 : nitems - 2); |
| 622 | break; | 619 | break; |
| 623 | - } else if (nkids > 0) { | 620 | + } |
| 621 | + | ||
| 622 | + auto kids = a_node.getKey("/Kids"); | ||
| 623 | + int nkids = kids.isArray() ? static_cast<int>(kids.size()) : 0; | ||
| 624 | + if (nkids > 0) { | ||
| 624 | int kid_number = first ? 0 : nkids - 1; | 625 | int kid_number = first ? 0 : nkids - 1; |
| 625 | addPathElement(a_node, kid_number); | 626 | addPathElement(a_node, kid_number); |
| 626 | - auto next = kids.getArrayItem(kid_number); | 627 | + auto next = kids[kid_number]; |
| 627 | if (!next.isIndirect()) { | 628 | if (!next.isIndirect()) { |
| 628 | if (impl.auto_repair) { | 629 | if (impl.auto_repair) { |
| 629 | - QTC::TC("qpdf", "NNTree fix indirect kid"); | ||
| 630 | impl.warn( | 630 | impl.warn( |
| 631 | a_node, | 631 | a_node, |
| 632 | - ("converting kid number " + std::to_string(kid_number) + | ||
| 633 | - " to an indirect object")); | 632 | + "converting kid number " + std::to_string(kid_number) + |
| 633 | + " to an indirect object"); | ||
| 634 | next = impl.qpdf.makeIndirectObject(next); | 634 | next = impl.qpdf.makeIndirectObject(next); |
| 635 | kids.setArrayItem(kid_number, next); | 635 | kids.setArrayItem(kid_number, next); |
| 636 | } else { | 636 | } else { |
| 637 | - QTC::TC("qpdf", "NNTree warn indirect kid"); | ||
| 638 | impl.warn( | 637 | impl.warn( |
| 639 | a_node, | 638 | a_node, |
| 640 | - ("kid number " + std::to_string(kid_number) + | ||
| 641 | - " is not an indirect object")); | 639 | + "kid number " + std::to_string(kid_number) + " is not an indirect object"); |
| 642 | } | 640 | } |
| 643 | } | 641 | } |
| 644 | a_node = next; | 642 | a_node = next; |
| 645 | } else if (allow_empty && items.isArray()) { | 643 | } else if (allow_empty && items.isArray()) { |
| 646 | - QTC::TC("qpdf", "NNTree deepen found empty"); | ||
| 647 | setItemNumber(a_node, -1); | 644 | setItemNumber(a_node, -1); |
| 648 | break; | 645 | break; |
| 649 | } else { | 646 | } else { |
| 650 | - QTC::TC("qpdf", "NNTree deepen: invalid node"); | ||
| 651 | - impl.warn( | 647 | + return fail( |
| 652 | a_node, | 648 | a_node, |
| 653 | - ("name/number tree node has neither non-empty " + impl.details.itemsKey() + | ||
| 654 | - " nor /Kids")); | ||
| 655 | - failed = true; | ||
| 656 | - break; | 649 | + "name/number tree node has neither non-empty " + impl.details.itemsKey() + |
| 650 | + " nor /Kids"); | ||
| 657 | } | 651 | } |
| 658 | } | 652 | } |
| 659 | - if (failed) { | ||
| 660 | - path = opath; | ||
| 661 | - return false; | ||
| 662 | - } | ||
| 663 | return true; | 653 | return true; |
| 664 | } | 654 | } |
| 665 | 655 |
qpdf/qpdf.testcov
| @@ -519,15 +519,10 @@ qpdf-c called qpdf_oh_unparse_resolved 0 | @@ -519,15 +519,10 @@ qpdf-c called qpdf_oh_unparse_resolved 0 | ||
| 519 | qpdf-c called qpdf_oh_unparse_binary 0 | 519 | qpdf-c called qpdf_oh_unparse_binary 0 |
| 520 | QPDFWriter getFilterOnWrite false 0 | 520 | QPDFWriter getFilterOnWrite false 0 |
| 521 | QPDFPageObjectHelper::forEachXObject 3 | 521 | QPDFPageObjectHelper::forEachXObject 3 |
| 522 | -NNTree deepen: invalid node 0 | ||
| 523 | -NNTree deepen: loop 0 | ||
| 524 | NNTree skip invalid kid 0 | 522 | NNTree skip invalid kid 0 |
| 525 | NNTree skip item at end of short items 0 | 523 | NNTree skip item at end of short items 0 |
| 526 | NNTree skip invalid key 0 | 524 | NNTree skip invalid key 0 |
| 527 | -NNTree deepen found empty 0 | ||
| 528 | NNTree unable to determine limits 0 | 525 | NNTree unable to determine limits 0 |
| 529 | -NNTree warn indirect kid 0 | ||
| 530 | -NNTree fix indirect kid 0 | ||
| 531 | NNTree repair 0 | 526 | NNTree repair 0 |
| 532 | NNTree split root + leaf 0 | 527 | NNTree split root + leaf 0 |
| 533 | NNTree split root + !leaf 0 | 528 | NNTree split root + !leaf 0 |
| @@ -536,7 +531,6 @@ NNTree split items 0 | @@ -536,7 +531,6 @@ NNTree split items 0 | ||
| 536 | NNTree split second half item 0 | 531 | NNTree split second half item 0 |
| 537 | NNTree split parent 0 | 532 | NNTree split parent 0 |
| 538 | NNTree split second half kid 0 | 533 | NNTree split second half kid 0 |
| 539 | -NNTree node is not a dictionary 0 | ||
| 540 | NNTree limits didn't change 0 | 534 | NNTree limits didn't change 0 |
| 541 | NNTree increment end() 0 | 535 | NNTree increment end() 0 |
| 542 | NNTree insertAfter inserts first 0 | 536 | NNTree insertAfter inserts first 0 |