Commit 90cb87f937f7c2e47351979b14f5e63ca18763b0

Authored by m-holger
Committed by GitHub
2 parents f3063577 a750b8b9

Merge pull request #1606 from m-holger/cover

Refactor error handling in `Pipeline` and `QPDFXRefEntry`
libqpdf/CMakeLists.txt
@@ -83,7 +83,6 @@ set(libqpdf_SOURCES @@ -83,7 +83,6 @@ set(libqpdf_SOURCES
83 QPDFTokenizer.cc 83 QPDFTokenizer.cc
84 QPDFUsage.cc 84 QPDFUsage.cc
85 QPDFWriter.cc 85 QPDFWriter.cc
86 - QPDFXRefEntry.cc  
87 QPDF_Array.cc 86 QPDF_Array.cc
88 QPDF_Dictionary.cc 87 QPDF_Dictionary.cc
89 QPDF_Stream.cc 88 QPDF_Stream.cc
libqpdf/Pipeline.cc
1 #include <qpdf/Pipeline.hh> 1 #include <qpdf/Pipeline.hh>
2 2
  3 +#include <qpdf/Util.hh>
  4 +
3 #include <cstring> 5 #include <cstring>
4 #include <stdexcept> 6 #include <stdexcept>
5 7
  8 +using namespace qpdf;
  9 +
6 Pipeline::Pipeline(char const* identifier, Pipeline* next) : 10 Pipeline::Pipeline(char const* identifier, Pipeline* next) :
7 identifier(identifier), 11 identifier(identifier),
8 next_(next) 12 next_(next)
@@ -12,10 +16,8 @@ Pipeline::Pipeline(char const* identifier, Pipeline* next) : @@ -12,10 +16,8 @@ Pipeline::Pipeline(char const* identifier, Pipeline* next) :
12 Pipeline* 16 Pipeline*
13 Pipeline::getNext(bool allow_null) 17 Pipeline::getNext(bool allow_null)
14 { 18 {
15 - if (!next_ && !allow_null) {  
16 - throw std::logic_error(  
17 - identifier + ": Pipeline::getNext() called on pipeline with no next");  
18 - } 19 + util::assertion(
  20 + next_ || allow_null, identifier + ": Pipeline::getNext() called on pipeline with no next");
19 return next_; 21 return next_;
20 } 22 }
21 23
libqpdf/QPDFXRefEntry.cc deleted
1 -#include <qpdf/QPDFXRefEntry.hh>  
2 -  
3 -#include <qpdf/QIntC.hh>  
4 -#include <qpdf/QPDFExc.hh>  
5 -  
6 -QPDFXRefEntry::QPDFXRefEntry() // NOLINT (modernize-use-equals-default)  
7 -{  
8 -}  
9 -  
10 -QPDFXRefEntry::QPDFXRefEntry(int type, qpdf_offset_t field1, int field2) :  
11 - type(type),  
12 - field1(field1),  
13 - field2(field2)  
14 -{  
15 - if ((type < 1) || (type > 2)) {  
16 - throw std::logic_error("invalid xref type " + std::to_string(type));  
17 - }  
18 -}  
19 -  
20 -int  
21 -QPDFXRefEntry::getType() const  
22 -{  
23 - return this->type;  
24 -}  
25 -  
26 -qpdf_offset_t  
27 -QPDFXRefEntry::getOffset() const  
28 -{  
29 - if (this->type != 1) {  
30 - throw std::logic_error("getOffset called for xref entry of type != 1");  
31 - }  
32 - return this->field1;  
33 -}  
34 -  
35 -int  
36 -QPDFXRefEntry::getObjStreamNumber() const  
37 -{  
38 - if (this->type != 2) {  
39 - throw std::logic_error("getObjStreamNumber called for xref entry of type != 2");  
40 - }  
41 - return QIntC::to_int(this->field1);  
42 -}  
43 -  
44 -int  
45 -QPDFXRefEntry::getObjStreamIndex() const  
46 -{  
47 - if (this->type != 2) {  
48 - throw std::logic_error("getObjStreamIndex called for xref entry of type != 2");  
49 - }  
50 - return this->field2;  
51 -}  
libqpdf/QPDF_objects.cc
@@ -26,6 +26,43 @@ using namespace std::literals; @@ -26,6 +26,43 @@ using namespace std::literals;
26 26
27 using Objects = QPDF::Doc::Objects; 27 using Objects = QPDF::Doc::Objects;
28 28
  29 +QPDFXRefEntry::QPDFXRefEntry() = default;
  30 +
  31 +QPDFXRefEntry::QPDFXRefEntry(int type, qpdf_offset_t field1, int field2) :
  32 + type(type),
  33 + field1(field1),
  34 + field2(field2)
  35 +{
  36 + util::assertion(type == 1 || type == 2, "invalid xref type " + std::to_string(type));
  37 +}
  38 +
  39 +int
  40 +QPDFXRefEntry::getType() const
  41 +{
  42 + return type;
  43 +}
  44 +
  45 +qpdf_offset_t
  46 +QPDFXRefEntry::getOffset() const
  47 +{
  48 + util::assertion(type == 1, "getOffset called for xref entry of type != 1");
  49 + return this->field1;
  50 +}
  51 +
  52 +int
  53 +QPDFXRefEntry::getObjStreamNumber() const
  54 +{
  55 + util::assertion(type == 2, "getObjStreamNumber called for xref entry of type != 2");
  56 + return QIntC::to_int(field1);
  57 +}
  58 +
  59 +int
  60 +QPDFXRefEntry::getObjStreamIndex() const
  61 +{
  62 + util::assertion(type == 2, "getObjStreamIndex called for xref entry of type != 2");
  63 + return field2;
  64 +}
  65 +
29 namespace 66 namespace
30 { 67 {
31 class InvalidInputSource: public InputSource 68 class InvalidInputSource: public InputSource
qpdf/qtest/misc.test 0 → 100644
  1 +#!/usr/bin/env perl
  2 +require 5.008;
  3 +use warnings;
  4 +use strict;
  5 +
  6 +unshift(@INC, '.');
  7 +require qpdf_test_helpers;
  8 +
  9 +chdir("qpdf") or die "chdir testdir failed: $!\n";
  10 +
  11 +require TestDriver;
  12 +
  13 +my $dev_null = File::Spec->devnull();
  14 +cleanup();
  15 +
  16 +my $td = new TestDriver('misc');
  17 +
  18 +my $n_tests = 1;
  19 +
  20 +$td->runtest("pipeline",
  21 + {$td->COMMAND => "test_driver 62 -"},
  22 + {$td->STRING => "test 62 done\n",
  23 + $td->EXIT_STATUS => 0},
  24 + $td->NORMALIZE_NEWLINES);
  25 +
  26 +cleanup();
  27 +$td->report($n_tests);
qpdf/test_driver.cc
@@ -2279,8 +2279,17 @@ test_61(QPDF&amp; pdf, char const* arg2) @@ -2279,8 +2279,17 @@ test_61(QPDF&amp; pdf, char const* arg2)
2279 static void 2279 static void
2280 test_62(QPDF& pdf, char const* arg2) 2280 test_62(QPDF& pdf, char const* arg2)
2281 { 2281 {
2282 - // Test int size checks. This test will fail if int and long long are the same size.  
2283 - // Moved to libtests/objects 2282 + // Was test int size checks - Moved to libtests/objects.
  2283 + // Test Pipeline methods
  2284 + std::string out;
  2285 + Pl_String pl("", nullptr, out);
  2286 + unsigned short us = 1;
  2287 + unsigned int ui = 2;
  2288 + unsigned long long ull = 3;
  2289 + long l = 4;
  2290 + short s = 5;
  2291 + pl << us << ui << ull << l << s;
  2292 + assert(out == "12345");
2284 } 2293 }
2285 2294
2286 static void 2295 static void
@@ -3548,7 +3557,7 @@ runtest(int n, char const* filename1, char const* arg2) @@ -3548,7 +3557,7 @@ runtest(int n, char const* filename1, char const* arg2)
3548 // the test suite to see how the test is invoked to find the file 3557 // the test suite to see how the test is invoked to find the file
3549 // that the test is supposed to operate on. 3558 // that the test is supposed to operate on.
3550 3559
3551 - std::set<int> ignore_filename = {61, 81, 83, 84, 85, 86, 87, 92, 95, 96}; 3560 + std::set<int> ignore_filename = {61, 62, 81, 83, 84, 85, 86, 87, 92, 95, 96};
3552 3561
3553 if (n == 0) { 3562 if (n == 0) {
3554 // Throw in some random test cases that don't fit anywhere 3563 // Throw in some random test cases that don't fit anywhere