OpenSystemsDevelopment / qpdf

Commit 5f3f78822b5d43e9b02082da5268d186ba7101c0

Authored by Jay Berkenbilt
1 parent 88c3d556

Improve use of std::unique_ptr 

* Use unique_ptr in place of shared_ptr in some cases
* unique_ptr for arrays does not require a custom deleter
* use std::make_unique (c++14) where possible
Inline Side-by-side
Showing 14 changed files with 54 additions and 23 deletions
ChangeLog
  View file @5f3f788
  1 +2022-02-05 Jay Berkenbilt <ejb@ql.org>
  2 +
  3 + * Add QUtil::make_unique_cstr to return a std::unique_ptr<char[]>
  4 + as an alternative to QUtil::copy_string and
  5 + QUtil::make_shared_cstr.
  6 +
1 7 2022-02-04 Jay Berkenbilt <ejb@ql.org>
2 8  
3 9 * New preprocessor symbols QPDF_MAJOR_VERSION, QPDF_MINOR_VERSION,
... ...
1 1 10.6
2 2 ====
3 3  
  4 +* Expose emptyPDF to the C API. Ensure that qpdf_get_qpdf_version is
  5 + always static.
  6 +
4 7 * Consider doing one big commit to reformat the entire codebase using
5 8 clang-format or a similar tool. Consider using blame.ignoreRevsFile
6 9 or similar (or otherwise study git blame to see how to minimize the
... ... @@ -364,6 +367,13 @@ auto x_ph = std::make_shared&lt;X&gt;(); X* x = x_ph.get();
364 367 Derived* x = new Derived(); PointerHolder<Base> x_ph(x) -->
365 368 Derived* x = new Derived(); auto x_ph = std::shared_pointer<Base>(x);
366 369  
  370 +Also remember
  371 +
  372 +auto x = std::shared_ptr(new T[5], std::default_delete<T[]>())
  373 +vs.
  374 +auto x = std::make_unique<T[]>(5)
  375 +
  376 +
367 377 PointerHolder in public API:
368 378  
369 379 QUtil::read_file_into_memory(
... ...
include/qpdf/QUtil.hh
  View file @5f3f788
... ... @@ -161,6 +161,10 @@ namespace QUtil
161 161 QPDF_DLL
162 162 std::shared_ptr<char> make_shared_cstr(std::string const&);
163 163  
  164 + // Copy string as a unique_ptr to an array.
  165 + QPDF_DLL
  166 + std::unique_ptr<char[]> make_unique_cstr(std::string const&);
  167 +
164 168 // Returns lower-case hex-encoded version of the string, treating
165 169 // each character in the input string as unsigned. The output
166 170 // string will be twice as long as the input string.
... ...
libqpdf/AES_PDF_native.cc
  View file @5f3f788
... ... @@ -19,12 +19,8 @@ AES_PDF_native::AES_PDF_native(bool encrypt, unsigned char const* key,
19 19 nrounds(0)
20 20 {
21 21 size_t keybits = 8 * key_bytes;
22   - this->key = std::unique_ptr<unsigned char[]>(
23   - new unsigned char[key_bytes],
24   - std::default_delete<unsigned char[]>());
25   - this->rk = std::unique_ptr<uint32_t[]>(
26   - new uint32_t[RKLENGTH(keybits)],
27   - std::default_delete<uint32_t[]>());
  22 + this->key = std::make_unique<unsigned char[]>(key_bytes);
  23 + this->rk = std::make_unique<uint32_t[]>(RKLENGTH(keybits));
28 24 size_t rk_bytes = RKLENGTH(keybits) * sizeof(uint32_t);
29 25 std::memcpy(this->key.get(), key, key_bytes);
30 26 std::memset(this->rk.get(), 0, rk_bytes);
... ...
libqpdf/Pl_AES_PDF.cc
  View file @5f3f788
... ... @@ -25,9 +25,7 @@ Pl_AES_PDF::Pl_AES_PDF(char const* identifier, Pipeline* next,
25 25 use_specified_iv(false),
26 26 disable_padding(false)
27 27 {
28   - this->key = std::unique_ptr<unsigned char[]>(
29   - new unsigned char[key_bytes],
30   - std::default_delete<unsigned char[]>());
  28 + this->key = std::make_unique<unsigned char[]>(key_bytes);
31 29 std::memcpy(this->key.get(), key, key_bytes);
32 30 std::memset(this->inbuf, 0, this->buf_size);
33 31 std::memset(this->outbuf, 0, this->buf_size);
... ...
libqpdf/QPDFArgParser.cc
  View file @5f3f788
... ... @@ -20,7 +20,7 @@ QPDFArgParser::Members::Members(
20 20 option_table(nullptr),
21 21 final_check_handler(nullptr)
22 22 {
23   - auto tmp = QUtil::make_shared_cstr(argv[0]);
  23 + auto tmp = QUtil::make_unique_cstr(argv[0]);
24 24 char* p = QUtil::getWhoami(tmp.get());
25 25 // Remove prefix added by libtool for consistency during testing.
26 26 if (strncmp(p, "lt-", 3) == 0)
... ...
libqpdf/QPDFJob.cc
  View file @5f3f788
... ... @@ -3379,7 +3379,7 @@ QPDFJob::setEncryptionOptions(QPDF&amp; pdf, QPDFWriter&amp; w)
3379 3379 static void parse_version(std::string const& full_version_string,
3380 3380 std::string& version, int& extension_level)
3381 3381 {
3382   - auto vp = QUtil::make_shared_cstr(full_version_string);
  3382 + auto vp = QUtil::make_unique_cstr(full_version_string);
3383 3383 char* v = vp.get();
3384 3384 char* p1 = strchr(v, '.');
3385 3385 char* p2 = (p1 ? strchr(1 + p1, '.') : 0);
... ...
libqpdf/QPDFWriter.cc
  View file @5f3f788
... ... @@ -1919,7 +1919,7 @@ QPDFWriter::unparseObject(QPDFObjectHandle object, int level,
1919 1919 }
1920 1920 else
1921 1921 {
1922   - auto tmp_ph = QUtil::make_shared_cstr(val);
  1922 + auto tmp_ph = QUtil::make_unique_cstr(val);
1923 1923 char* tmp = tmp_ph.get();
1924 1924 size_t vlen = val.length();
1925 1925 RC4 rc4(QUtil::unsigned_char_pointer(this->m->cur_data_key),
... ...
libqpdf/QPDF_encryption.cc
  View file @5f3f788
... ... @@ -1211,7 +1211,7 @@ QPDF::decryptString(std::string&amp; str, int objid, int generation)
1211 1211 size_t vlen = str.length();
1212 1212 // Using PointerHolder guarantees that tmp will
1213 1213 // be freed even if rc4.process throws an exception.
1214   - auto tmp = QUtil::make_shared_cstr(str);
  1214 + auto tmp = QUtil::make_unique_cstr(str);
1215 1215 RC4 rc4(QUtil::unsigned_char_pointer(key), toI(key.length()));
1216 1216 rc4.process(QUtil::unsigned_char_pointer(tmp.get()), vlen);
1217 1217 str = std::string(tmp.get(), vlen);
... ...
libqpdf/QUtil.cc
  View file @5f3f788
... ... @@ -744,6 +744,16 @@ QUtil::make_shared_cstr(std::string const&amp; str)
744 744 return result;
745 745 }
746 746  
  747 +std::unique_ptr<char[]>
  748 +QUtil::make_unique_cstr(std::string const& str)
  749 +{
  750 + auto result = std::make_unique<char[]>(str.length() + 1);
  751 + // Use memcpy in case string contains nulls
  752 + result.get()[str.length()] = '\0';
  753 + memcpy(result.get(), str.c_str(), str.length());
  754 + return result;
  755 +}
  756 +
747 757 std::string
748 758 QUtil::hex_encode(std::string const& input)
749 759 {
... ... @@ -2625,7 +2635,7 @@ call_main_from_wmain(bool, int argc, wchar_t const* const argv[],
2625 2635 // other systems. That way the rest of qpdf.cc can just act like
2626 2636 // arguments are UTF-8.
2627 2637  
2628   - std::vector<std::shared_ptr<char>> utf8_argv;
  2638 + std::vector<std::unique_ptr<char[]>> utf8_argv;
2629 2639 for (int i = 0; i < argc; ++i)
2630 2640 {
2631 2641 std::string utf16;
... ... @@ -2638,11 +2648,9 @@ call_main_from_wmain(bool, int argc, wchar_t const* const argv[],
2638 2648 QIntC::to_uchar(codepoint & 0xff)));
2639 2649 }
2640 2650 std::string utf8 = QUtil::utf16_to_utf8(utf16);
2641   - utf8_argv.push_back(QUtil::make_shared_cstr(utf8));
  2651 + utf8_argv.push_back(QUtil::make_unique_cstr(utf8));
2642 2652 }
2643   - auto utf8_argv_sp =
2644   - std::shared_ptr<char*>(
2645   - new char*[1+utf8_argv.size()], std::default_delete<char*[]>());
  2653 + auto utf8_argv_sp = std::make_unique<char*[]>(1+utf8_argv.size());
2646 2654 char** new_argv = utf8_argv_sp.get();
2647 2655 for (size_t i = 0; i < utf8_argv.size(); ++i)
2648 2656 {
... ...
libqpdf/qpdfjob-c.cc
  View file @5f3f788
... ... @@ -9,7 +9,7 @@
9 9  
10 10 int qpdfjob_run_from_argv(char const* const argv[])
11 11 {
12   - auto whoami_p = QUtil::make_shared_cstr(argv[0]);
  12 + auto whoami_p = QUtil::make_unique_cstr(argv[0]);
13 13 auto whoami = QUtil::getWhoami(whoami_p.get());
14 14 QUtil::setLineBuf(stdout);
15 15  
... ...
libtests/qtest/qutil/qutil.out
  View file @5f3f788
... ... @@ -23,6 +23,7 @@ one
23 23 7
24 24 compare okay
25 25 compare okay
  26 +compare okay
26 27 -2147483648 to int: PASSED
27 28 2147483647 to int: PASSED
28 29 2147483648 to int threw (integer out of range converting 2147483648 from a 8-byte signed type to a 4-byte signed type): PASSED
... ...
libtests/qutil.cc
  View file @5f3f788
... ... @@ -150,7 +150,7 @@ void string_conversion_test()
150 150 std::cout << "compare failed" << std::endl;
151 151 }
152 152 delete [] tmp;
153   - // Also test with make_shared_cstr
  153 + // Also test with make_shared_cstr and make_unique_cstr
154 154 auto tmp2 = QUtil::make_shared_cstr(embedded_null);
155 155 if (memcmp(tmp2.get(), embedded_null.c_str(), 7) == 0)
156 156 {
... ... @@ -160,6 +160,15 @@ void string_conversion_test()
160 160 {
161 161 std::cout << "compare failed" << std::endl;
162 162 }
  163 + auto tmp3 = QUtil::make_unique_cstr(embedded_null);
  164 + if (memcmp(tmp3.get(), embedded_null.c_str(), 7) == 0)
  165 + {
  166 + std::cout << "compare okay" << std::endl;
  167 + }
  168 + else
  169 + {
  170 + std::cout << "compare failed" << std::endl;
  171 + }
163 172  
164 173 std::string int_max_str = QUtil::int_to_string(INT_MAX);
165 174 std::string int_min_str = QUtil::int_to_string(INT_MIN);
... ... @@ -417,7 +426,7 @@ void transcoding_test()
417 426  
418 427 void print_whoami(char const* str)
419 428 {
420   - auto dup = QUtil::make_shared_cstr(str);
  429 + auto dup = QUtil::make_unique_cstr(str);
421 430 std::cout << QUtil::getWhoami(dup.get()) << std::endl;
422 431 }
423 432  
... ...
libtests/rc4.cc
  View file @5f3f788
... ... @@ -14,8 +14,7 @@ static void other_tests()
14 14 // Test cases not covered by the pipeline: string as key, convert
15 15 // in place
16 16 RC4 r(reinterpret_cast<unsigned char const*>("quack"));
17   - auto data = std::unique_ptr<unsigned char[]>(
18   - new unsigned char[6], std::default_delete<unsigned char[]>());
  17 + auto data = std::make_unique<unsigned char[]>(6);
19 18 memcpy(data.get(), "potato", 6);
20 19 r.process(data.get(), 6);
21 20 assert(memcmp(data.get(), "\xa5\x6f\xe7\x27\x2b\x5c", 6) == 0);
... ...