Commit 76b1659177327a64037bf36d7f3e15a73d86bbed
1 parent
14fe2e6d
enhance PointerHolder so that it can explicitly be told to use delete [] instead…
… of delete, thus making it useful to run valgrind over qpdf during its test suite
Showing
13 changed files
with
153 additions
and
49 deletions
README.maintainer
| 1 | 1 | Release Reminders |
| 2 | 2 | ================= |
| 3 | 3 | |
| 4 | + * Consider running tests with latest gcc and/or valgrind. To do | |
| 5 | + this, replace, build with debugging and without shared libraries. | |
| 6 | + In build, create z and move each executable into z. Then create a | |
| 7 | + script called exec-z that contains: | |
| 8 | + | |
| 9 | + #!/bin/sh | |
| 10 | + exec valgrind --suppressions=/tmp/a.supp -q \ | |
| 11 | + `dirname $0`/z/`basename $0` ${1+"$@"} | |
| 12 | + | |
| 13 | + Symlink exec-v to each executable. /tmp/a.supp can be populated | |
| 14 | + with suppressions for libraries, for example: | |
| 15 | + | |
| 16 | + { | |
| 17 | + zlib1 | |
| 18 | + Memcheck:Cond | |
| 19 | + fun:inflateReset2 | |
| 20 | + fun:inflateInit2_ | |
| 21 | + } | |
| 22 | + | |
| 23 | + You can generate these by running valgrind with --gen-suppressions=yes. | |
| 24 | + | |
| 4 | 25 | * Check all open issues in the sourceforge trackers. |
| 5 | 26 | |
| 6 | 27 | * If any interfaces were added or changed, check C API to see whether | ... | ... |
TODO
include/qpdf/PointerHolder.hh
| ... | ... | @@ -8,8 +8,6 @@ |
| 8 | 8 | #ifndef __POINTERHOLDER_HH__ |
| 9 | 9 | #define __POINTERHOLDER_HH__ |
| 10 | 10 | |
| 11 | -#include <iostream> | |
| 12 | - | |
| 13 | 11 | // This class is basically boost::shared_pointer but predates that by |
| 14 | 12 | // several years. |
| 15 | 13 | |
| ... | ... | @@ -45,43 +43,42 @@ class PointerHolder |
| 45 | 43 | class Data |
| 46 | 44 | { |
| 47 | 45 | public: |
| 48 | - Data(T* pointer, bool tracing) : | |
| 46 | + Data(T* pointer, bool array) : | |
| 49 | 47 | pointer(pointer), |
| 50 | - tracing(tracing), | |
| 48 | + array(array), | |
| 51 | 49 | refcount(0) |
| 52 | 50 | { |
| 53 | - static int next_id = 0; | |
| 54 | - this->unique_id = ++next_id; | |
| 55 | 51 | } |
| 56 | 52 | ~Data() |
| 57 | 53 | { |
| 58 | - if (this->tracing) | |
| 54 | + if (array) | |
| 59 | 55 | { |
| 60 | - std::cerr << "PointerHolder deleting pointer " | |
| 61 | - << (void*)pointer | |
| 62 | - << std::endl; | |
| 56 | + delete [] this->pointer; | |
| 63 | 57 | } |
| 64 | - delete this->pointer; | |
| 65 | - if (this->tracing) | |
| 58 | + else | |
| 66 | 59 | { |
| 67 | - std::cerr << "PointerHolder done deleting pointer " | |
| 68 | - << (void*)pointer | |
| 69 | - << std::endl; | |
| 60 | + delete this->pointer; | |
| 70 | 61 | } |
| 71 | 62 | } |
| 72 | 63 | T* pointer; |
| 73 | - bool tracing; | |
| 64 | + bool array; | |
| 74 | 65 | int refcount; |
| 75 | - int unique_id; | |
| 76 | 66 | private: |
| 77 | 67 | Data(Data const&); |
| 78 | 68 | Data& operator=(Data const&); |
| 79 | 69 | }; |
| 80 | 70 | |
| 81 | 71 | public: |
| 72 | + // "tracing" is not used but is kept for interface backward compatbility | |
| 82 | 73 | PointerHolder(T* pointer = 0, bool tracing = false) |
| 83 | 74 | { |
| 84 | - this->init(new Data(pointer, tracing)); | |
| 75 | + this->init(new Data(pointer, false)); | |
| 76 | + } | |
| 77 | + // Special constructor indicating to free memory with delete [] | |
| 78 | + // instead of delete | |
| 79 | + PointerHolder(bool, T* pointer) | |
| 80 | + { | |
| 81 | + this->init(new Data(pointer, true)); | |
| 85 | 82 | } |
| 86 | 83 | PointerHolder(PointerHolder const& rhs) |
| 87 | 84 | { |
| ... | ... | @@ -148,12 +145,6 @@ class PointerHolder |
| 148 | 145 | this->data = data; |
| 149 | 146 | { |
| 150 | 147 | ++this->data->refcount; |
| 151 | - if (this->data->tracing) | |
| 152 | - { | |
| 153 | - std::cerr << "PointerHolder " << this->data->unique_id | |
| 154 | - << " refcount increased to " << this->data->refcount | |
| 155 | - << std::endl; | |
| 156 | - } | |
| 157 | 148 | } |
| 158 | 149 | } |
| 159 | 150 | void copy(PointerHolder const& rhs) |
| ... | ... | @@ -168,13 +159,6 @@ class PointerHolder |
| 168 | 159 | { |
| 169 | 160 | gone = true; |
| 170 | 161 | } |
| 171 | - if (this->data->tracing) | |
| 172 | - { | |
| 173 | - std::cerr << "PointerHolder " << this->data->unique_id | |
| 174 | - << " refcount decreased to " | |
| 175 | - << this->data->refcount | |
| 176 | - << std::endl; | |
| 177 | - } | |
| 178 | 162 | } |
| 179 | 163 | if (gone) |
| 180 | 164 | { | ... | ... |
include/qpdf/qpdf-c.h
| ... | ... | @@ -285,6 +285,25 @@ extern "C" { |
| 285 | 285 | QPDF_DLL |
| 286 | 286 | QPDF_ERROR_CODE qpdf_init_write(qpdf_data qpdf, char const* filename); |
| 287 | 287 | |
| 288 | + /* Initialize for writing but indicate that the PDF file should be | |
| 289 | + * written to memory. Call qpdf_get_buffer_length and | |
| 290 | + * qpdf_get_buffer to retrieve the resulting buffer. The memory | |
| 291 | + * containing the PDF file will be destroyed when qpdf_cleanup is | |
| 292 | + * called. | |
| 293 | + */ | |
| 294 | + QPDF_DLL | |
| 295 | + QPDF_ERROR_CODE qpdf_init_write_memory(qpdf_data qpdf); | |
| 296 | + | |
| 297 | + /* Retrieve the buffer used if the file was written to memory. | |
| 298 | + * qpdf_get_buffer returns a null pointer if data was not written | |
| 299 | + * to memory. The memory is freed when qpdf_cleanup is called or | |
| 300 | + * if a subsequent call to qpdf_init_write or | |
| 301 | + * qpdf_init_write_memory is called. */ | |
| 302 | + QPDF_DLL | |
| 303 | + unsigned long qpdf_get_buffer_length(qpdf_data qpdf); | |
| 304 | + QPDF_DLL | |
| 305 | + unsigned char const* qpdf_get_buffer(qpdf_data qpdf); | |
| 306 | + | |
| 288 | 307 | QPDF_DLL |
| 289 | 308 | void qpdf_set_object_stream_mode(qpdf_data qpdf, |
| 290 | 309 | enum qpdf_object_stream_e mode); | ... | ... |
libqpdf/QPDF.cc
| ... | ... | @@ -403,10 +403,9 @@ QPDF::parse(char const* password) |
| 403 | 403 | this->file->rewind(); |
| 404 | 404 | } |
| 405 | 405 | char* buf = new char[tbuf_size + 1]; |
| 406 | - // Put buf in a PointerHolder to guarantee deletion of buf. This | |
| 407 | - // calls delete rather than delete [], but it's okay since buf is | |
| 408 | - // an array of fundamental types. | |
| 409 | - PointerHolder<char> b(buf); | |
| 406 | + // Put buf in an array-style PointerHolder to guarantee deletion | |
| 407 | + // of buf. | |
| 408 | + PointerHolder<char> b(true, buf); | |
| 410 | 409 | memset(buf, '\0', tbuf_size + 1); |
| 411 | 410 | this->file->read(buf, tbuf_size); |
| 412 | 411 | ... | ... |
libqpdf/QPDF_encryption.cc
| ... | ... | @@ -589,12 +589,9 @@ QPDF::decryptString(std::string& str, int objid, int generation) |
| 589 | 589 | { |
| 590 | 590 | QTC::TC("qpdf", "QPDF_encryption rc4 decode string"); |
| 591 | 591 | unsigned int vlen = str.length(); |
| 592 | - // Using PointerHolder will cause a new char[] to be deleted | |
| 593 | - // with delete instead of delete [], but it's okay since the | |
| 594 | - // array is of a fundamental type, so there is no destructor | |
| 595 | - // to be called. Using PointerHolder guarantees that tmp will | |
| 592 | + // Using PointerHolder guarantees that tmp will | |
| 596 | 593 | // be freed even if rc4.process throws an exception. |
| 597 | - PointerHolder<char> tmp = QUtil::copy_string(str); | |
| 594 | + PointerHolder<char> tmp(true, QUtil::copy_string(str)); | |
| 598 | 595 | RC4 rc4((unsigned char const*)key.c_str(), key.length()); |
| 599 | 596 | rc4.process((unsigned char*)tmp.getPointer(), vlen); |
| 600 | 597 | str = std::string(tmp.getPointer(), vlen); | ... | ... |
libqpdf/QPDF_linearization.cc
| ... | ... | @@ -88,7 +88,7 @@ QPDF::isLinearized() |
| 88 | 88 | |
| 89 | 89 | char* buf = new char[tbuf_size]; |
| 90 | 90 | this->file->seek(0, SEEK_SET); |
| 91 | - PointerHolder<char> b(buf); // guarantee deletion | |
| 91 | + PointerHolder<char> b(true, buf); | |
| 92 | 92 | memset(buf, '\0', tbuf_size); |
| 93 | 93 | this->file->read(buf, tbuf_size - 1); |
| 94 | 94 | ... | ... |
libqpdf/qpdf-c.cc
| ... | ... | @@ -33,11 +33,15 @@ struct _qpdf_data |
| 33 | 33 | char const* buffer; |
| 34 | 34 | unsigned long size; |
| 35 | 35 | char const* password; |
| 36 | + bool write_memory; | |
| 37 | + Buffer* output_buffer; | |
| 36 | 38 | }; |
| 37 | 39 | |
| 38 | 40 | _qpdf_data::_qpdf_data() : |
| 39 | 41 | qpdf(0), |
| 40 | - qpdf_writer(0) | |
| 42 | + qpdf_writer(0), | |
| 43 | + write_memory(false), | |
| 44 | + output_buffer(0) | |
| 41 | 45 | { |
| 42 | 46 | } |
| 43 | 47 | |
| ... | ... | @@ -45,6 +49,7 @@ _qpdf_data::~_qpdf_data() |
| 45 | 49 | { |
| 46 | 50 | delete qpdf_writer; |
| 47 | 51 | delete qpdf; |
| 52 | + delete output_buffer; | |
| 48 | 53 | } |
| 49 | 54 | |
| 50 | 55 | // must set qpdf->filename and qpdf->password |
| ... | ... | @@ -66,6 +71,12 @@ static void call_init_write(qpdf_data qpdf) |
| 66 | 71 | qpdf->qpdf_writer = new QPDFWriter(*(qpdf->qpdf), qpdf->filename); |
| 67 | 72 | } |
| 68 | 73 | |
| 74 | +static void call_init_write_memory(qpdf_data qpdf) | |
| 75 | +{ | |
| 76 | + qpdf->qpdf_writer = new QPDFWriter(*(qpdf->qpdf)); | |
| 77 | + qpdf->qpdf_writer->setOutputMemory(); | |
| 78 | +} | |
| 79 | + | |
| 69 | 80 | static void call_write(qpdf_data qpdf) |
| 70 | 81 | { |
| 71 | 82 | qpdf->qpdf_writer->write(); |
| ... | ... | @@ -408,21 +419,71 @@ QPDF_BOOL qpdf_allow_modify_all(qpdf_data qpdf) |
| 408 | 419 | return qpdf->qpdf->allowModifyAll(); |
| 409 | 420 | } |
| 410 | 421 | |
| 411 | -QPDF_ERROR_CODE qpdf_init_write(qpdf_data qpdf, char const* filename) | |
| 422 | +static void qpdf_init_write_internal(qpdf_data qpdf) | |
| 412 | 423 | { |
| 413 | - QPDF_ERROR_CODE status = QPDF_SUCCESS; | |
| 414 | 424 | if (qpdf->qpdf_writer) |
| 415 | 425 | { |
| 416 | 426 | QTC::TC("qpdf", "qpdf-c called qpdf_init_write multiple times"); |
| 417 | 427 | delete qpdf->qpdf_writer; |
| 418 | 428 | qpdf->qpdf_writer = 0; |
| 429 | + if (qpdf->output_buffer) | |
| 430 | + { | |
| 431 | + delete qpdf->output_buffer; | |
| 432 | + qpdf->output_buffer = 0; | |
| 433 | + qpdf->write_memory = false; | |
| 434 | + qpdf->filename = 0; | |
| 435 | + } | |
| 419 | 436 | } |
| 437 | +} | |
| 438 | + | |
| 439 | +QPDF_ERROR_CODE qpdf_init_write(qpdf_data qpdf, char const* filename) | |
| 440 | +{ | |
| 441 | + qpdf_init_write_internal(qpdf); | |
| 420 | 442 | qpdf->filename = filename; |
| 421 | - status = trap_errors(qpdf, &call_init_write); | |
| 443 | + QPDF_ERROR_CODE status = trap_errors(qpdf, &call_init_write); | |
| 422 | 444 | QTC::TC("qpdf", "qpdf-c called qpdf_init_write", status); |
| 423 | 445 | return status; |
| 424 | 446 | } |
| 425 | 447 | |
| 448 | +QPDF_ERROR_CODE qpdf_init_write_memory(qpdf_data qpdf) | |
| 449 | +{ | |
| 450 | + qpdf_init_write_internal(qpdf); | |
| 451 | + QPDF_ERROR_CODE status = trap_errors(qpdf, &call_init_write_memory); | |
| 452 | + QTC::TC("qpdf", "qpdf-c called qpdf_init_write_memory"); | |
| 453 | + qpdf->write_memory = true; | |
| 454 | + return status; | |
| 455 | +} | |
| 456 | + | |
| 457 | +static void qpdf_get_buffer_internal(qpdf_data qpdf) | |
| 458 | +{ | |
| 459 | + if (qpdf->write_memory && (qpdf->output_buffer == 0)) | |
| 460 | + { | |
| 461 | + qpdf->output_buffer = qpdf->qpdf_writer->getBuffer(); | |
| 462 | + } | |
| 463 | +} | |
| 464 | + | |
| 465 | +unsigned long qpdf_get_buffer_length(qpdf_data qpdf) | |
| 466 | +{ | |
| 467 | + qpdf_get_buffer_internal(qpdf); | |
| 468 | + unsigned long result = 0L; | |
| 469 | + if (qpdf->output_buffer) | |
| 470 | + { | |
| 471 | + result = qpdf->output_buffer->getSize(); | |
| 472 | + } | |
| 473 | + return result; | |
| 474 | +} | |
| 475 | + | |
| 476 | +unsigned char const* qpdf_get_buffer(qpdf_data qpdf) | |
| 477 | +{ | |
| 478 | + unsigned char const* result = 0; | |
| 479 | + qpdf_get_buffer_internal(qpdf); | |
| 480 | + if (qpdf->output_buffer) | |
| 481 | + { | |
| 482 | + result = qpdf->output_buffer->getBuffer(); | |
| 483 | + } | |
| 484 | + return result; | |
| 485 | +} | |
| 486 | + | |
| 426 | 487 | void qpdf_set_object_stream_mode(qpdf_data qpdf, qpdf_object_stream_e mode) |
| 427 | 488 | { |
| 428 | 489 | QTC::TC("qpdf", "qpdf-c called qpdf_set_object_stream_mode"); | ... | ... |
libtests/buffer.cc
manual/qpdf-manual.xml
| ... | ... | @@ -2131,6 +2131,14 @@ print "\n"; |
| 2131 | 2131 | <literal>/Info</literal> dictionary. |
| 2132 | 2132 | </para> |
| 2133 | 2133 | </listitem> |
| 2134 | + <listitem> | |
| 2135 | + <para> | |
| 2136 | + Add functions <function>qpdf_init_write_memory</function>, | |
| 2137 | + <function>qpdf_get_buffer_length</function>, and | |
| 2138 | + <function>qpdf_get_buffer</function> to the C API for writing | |
| 2139 | + PDF files to a memory buffer instead of a file. | |
| 2140 | + </para> | |
| 2141 | + </listitem> | |
| 2134 | 2142 | </itemizedlist> |
| 2135 | 2143 | </listitem> |
| 2136 | 2144 | </varlistentry> | ... | ... |
qpdf/qpdf-ctest.c
| ... | ... | @@ -337,6 +337,10 @@ static void test16(char const* infile, |
| 337 | 337 | char const* outfile, |
| 338 | 338 | char const* outfile2) |
| 339 | 339 | { |
| 340 | + unsigned long buflen = 0L; | |
| 341 | + unsigned char const* buf = 0; | |
| 342 | + FILE* f = 0; | |
| 343 | + | |
| 340 | 344 | qpdf_read(qpdf, infile, password); |
| 341 | 345 | print_info("/Author"); |
| 342 | 346 | print_info("/Producer"); |
| ... | ... | @@ -347,11 +351,22 @@ static void test16(char const* infile, |
| 347 | 351 | print_info("/Author"); |
| 348 | 352 | print_info("/Producer"); |
| 349 | 353 | print_info("/Creator"); |
| 350 | - qpdf_init_write(qpdf, outfile); | |
| 354 | + qpdf_init_write_memory(qpdf); | |
| 351 | 355 | qpdf_set_static_ID(qpdf, QPDF_TRUE); |
| 352 | 356 | qpdf_set_static_aes_IV(qpdf, QPDF_TRUE); |
| 353 | 357 | qpdf_set_stream_data_mode(qpdf, qpdf_s_uncompress); |
| 354 | 358 | qpdf_write(qpdf); |
| 359 | + f = fopen(outfile, "wb"); | |
| 360 | + if (f == NULL) | |
| 361 | + { | |
| 362 | + fprintf(stderr, "%s: unable to open %s: %s\n", | |
| 363 | + whoami, outfile, strerror(errno)); | |
| 364 | + exit(2); | |
| 365 | + } | |
| 366 | + buflen = qpdf_get_buffer_length(qpdf); | |
| 367 | + buf = qpdf_get_buffer(qpdf); | |
| 368 | + fwrite(buf, 1, buflen, f); | |
| 369 | + fclose(f); | |
| 355 | 370 | report_errors(); |
| 356 | 371 | } |
| 357 | 372 | ... | ... |
qpdf/qpdf.testcov
qpdf/test_driver.cc
| ... | ... | @@ -76,7 +76,7 @@ void runtest(int n, char const* filename) |
| 76 | 76 | fseek(f, 0, SEEK_END); |
| 77 | 77 | size_t size = (size_t) ftell(f); |
| 78 | 78 | fseek(f, 0, SEEK_SET); |
| 79 | - file_buf = new char[size]; | |
| 79 | + file_buf = PointerHolder<char>(true, new char[size]); | |
| 80 | 80 | char* buf_p = file_buf.getPointer(); |
| 81 | 81 | size_t bytes_read = 0; |
| 82 | 82 | size_t len = 0; | ... | ... |