From 76b1659177327a64037bf36d7f3e15a73d86bbed Mon Sep 17 00:00:00 2001 From: Jay Berkenbilt Date: Thu, 11 Aug 2011 11:56:37 -0400 Subject: 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 --- README.maintainer | 21 +++++++++++++ TODO | 2 -- include/qpdf/PointerHolder.hh | 46 ++++++++++------------------- include/qpdf/qpdf-c.h | 19 ++++++++++++ libqpdf/QPDF.cc | 7 ++--- libqpdf/QPDF_encryption.cc | 7 ++--- libqpdf/QPDF_linearization.cc | 2 +- libqpdf/qpdf-c.cc | 69 ++++++++++++++++++++++++++++++++++++++++--- libtests/buffer.cc | 1 + manual/qpdf-manual.xml | 8 +++++ qpdf/qpdf-ctest.c | 17 ++++++++++- qpdf/qpdf.testcov | 1 + qpdf/test_driver.cc | 2 +- 13 files changed, 153 insertions(+), 49 deletions(-) diff --git a/README.maintainer b/README.maintainer index 7acf4849..f2791e0f 100644 --- a/README.maintainer +++ b/README.maintainer @@ -1,6 +1,27 @@ Release Reminders ================= + * Consider running tests with latest gcc and/or valgrind. To do + this, replace, build with debugging and without shared libraries. + In build, create z and move each executable into z. Then create a + script called exec-z that contains: + + #!/bin/sh + exec valgrind --suppressions=/tmp/a.supp -q \ + `dirname $0`/z/`basename $0` ${1+"$@"} + + Symlink exec-v to each executable. /tmp/a.supp can be populated + with suppressions for libraries, for example: + + { + zlib1 + Memcheck:Cond + fun:inflateReset2 + fun:inflateInit2_ + } + + You can generate these by running valgrind with --gen-suppressions=yes. + * Check all open issues in the sourceforge trackers. * If any interfaces were added or changed, check C API to see whether diff --git a/TODO b/TODO index 61848ba5..876c8365 100644 --- a/TODO +++ b/TODO @@ -3,8 +3,6 @@ * Provide an example of using replace and swap. Maybe. - * Add C API for writing to memory if possible - General ======= diff --git a/include/qpdf/PointerHolder.hh b/include/qpdf/PointerHolder.hh index b9bce8ee..b161c100 100644 --- a/include/qpdf/PointerHolder.hh +++ b/include/qpdf/PointerHolder.hh @@ -8,8 +8,6 @@ #ifndef __POINTERHOLDER_HH__ #define __POINTERHOLDER_HH__ -#include - // This class is basically boost::shared_pointer but predates that by // several years. @@ -45,43 +43,42 @@ class PointerHolder class Data { public: - Data(T* pointer, bool tracing) : + Data(T* pointer, bool array) : pointer(pointer), - tracing(tracing), + array(array), refcount(0) { - static int next_id = 0; - this->unique_id = ++next_id; } ~Data() { - if (this->tracing) + if (array) { - std::cerr << "PointerHolder deleting pointer " - << (void*)pointer - << std::endl; + delete [] this->pointer; } - delete this->pointer; - if (this->tracing) + else { - std::cerr << "PointerHolder done deleting pointer " - << (void*)pointer - << std::endl; + delete this->pointer; } } T* pointer; - bool tracing; + bool array; int refcount; - int unique_id; private: Data(Data const&); Data& operator=(Data const&); }; public: + // "tracing" is not used but is kept for interface backward compatbility PointerHolder(T* pointer = 0, bool tracing = false) { - this->init(new Data(pointer, tracing)); + this->init(new Data(pointer, false)); + } + // Special constructor indicating to free memory with delete [] + // instead of delete + PointerHolder(bool, T* pointer) + { + this->init(new Data(pointer, true)); } PointerHolder(PointerHolder const& rhs) { @@ -148,12 +145,6 @@ class PointerHolder this->data = data; { ++this->data->refcount; - if (this->data->tracing) - { - std::cerr << "PointerHolder " << this->data->unique_id - << " refcount increased to " << this->data->refcount - << std::endl; - } } } void copy(PointerHolder const& rhs) @@ -168,13 +159,6 @@ class PointerHolder { gone = true; } - if (this->data->tracing) - { - std::cerr << "PointerHolder " << this->data->unique_id - << " refcount decreased to " - << this->data->refcount - << std::endl; - } } if (gone) { diff --git a/include/qpdf/qpdf-c.h b/include/qpdf/qpdf-c.h index 98a6f07b..168da154 100644 --- a/include/qpdf/qpdf-c.h +++ b/include/qpdf/qpdf-c.h @@ -285,6 +285,25 @@ extern "C" { QPDF_DLL QPDF_ERROR_CODE qpdf_init_write(qpdf_data qpdf, char const* filename); + /* Initialize for writing but indicate that the PDF file should be + * written to memory. Call qpdf_get_buffer_length and + * qpdf_get_buffer to retrieve the resulting buffer. The memory + * containing the PDF file will be destroyed when qpdf_cleanup is + * called. + */ + QPDF_DLL + QPDF_ERROR_CODE qpdf_init_write_memory(qpdf_data qpdf); + + /* Retrieve the buffer used if the file was written to memory. + * qpdf_get_buffer returns a null pointer if data was not written + * to memory. The memory is freed when qpdf_cleanup is called or + * if a subsequent call to qpdf_init_write or + * qpdf_init_write_memory is called. */ + QPDF_DLL + unsigned long qpdf_get_buffer_length(qpdf_data qpdf); + QPDF_DLL + unsigned char const* qpdf_get_buffer(qpdf_data qpdf); + QPDF_DLL void qpdf_set_object_stream_mode(qpdf_data qpdf, enum qpdf_object_stream_e mode); diff --git a/libqpdf/QPDF.cc b/libqpdf/QPDF.cc index d614efe5..d161aebc 100644 --- a/libqpdf/QPDF.cc +++ b/libqpdf/QPDF.cc @@ -403,10 +403,9 @@ QPDF::parse(char const* password) this->file->rewind(); } char* buf = new char[tbuf_size + 1]; - // Put buf in a PointerHolder to guarantee deletion of buf. This - // calls delete rather than delete [], but it's okay since buf is - // an array of fundamental types. - PointerHolder b(buf); + // Put buf in an array-style PointerHolder to guarantee deletion + // of buf. + PointerHolder b(true, buf); memset(buf, '\0', tbuf_size + 1); this->file->read(buf, tbuf_size); diff --git a/libqpdf/QPDF_encryption.cc b/libqpdf/QPDF_encryption.cc index d559388b..772d87e0 100644 --- a/libqpdf/QPDF_encryption.cc +++ b/libqpdf/QPDF_encryption.cc @@ -589,12 +589,9 @@ QPDF::decryptString(std::string& str, int objid, int generation) { QTC::TC("qpdf", "QPDF_encryption rc4 decode string"); unsigned int vlen = str.length(); - // Using PointerHolder will cause a new char[] to be deleted - // with delete instead of delete [], but it's okay since the - // array is of a fundamental type, so there is no destructor - // to be called. Using PointerHolder guarantees that tmp will + // Using PointerHolder guarantees that tmp will // be freed even if rc4.process throws an exception. - PointerHolder tmp = QUtil::copy_string(str); + PointerHolder tmp(true, QUtil::copy_string(str)); RC4 rc4((unsigned char const*)key.c_str(), key.length()); rc4.process((unsigned char*)tmp.getPointer(), vlen); str = std::string(tmp.getPointer(), vlen); diff --git a/libqpdf/QPDF_linearization.cc b/libqpdf/QPDF_linearization.cc index 51b6ea6a..eeae05e9 100644 --- a/libqpdf/QPDF_linearization.cc +++ b/libqpdf/QPDF_linearization.cc @@ -88,7 +88,7 @@ QPDF::isLinearized() char* buf = new char[tbuf_size]; this->file->seek(0, SEEK_SET); - PointerHolder b(buf); // guarantee deletion + PointerHolder b(true, buf); memset(buf, '\0', tbuf_size); this->file->read(buf, tbuf_size - 1); diff --git a/libqpdf/qpdf-c.cc b/libqpdf/qpdf-c.cc index 62dba270..784351bc 100644 --- a/libqpdf/qpdf-c.cc +++ b/libqpdf/qpdf-c.cc @@ -33,11 +33,15 @@ struct _qpdf_data char const* buffer; unsigned long size; char const* password; + bool write_memory; + Buffer* output_buffer; }; _qpdf_data::_qpdf_data() : qpdf(0), - qpdf_writer(0) + qpdf_writer(0), + write_memory(false), + output_buffer(0) { } @@ -45,6 +49,7 @@ _qpdf_data::~_qpdf_data() { delete qpdf_writer; delete qpdf; + delete output_buffer; } // must set qpdf->filename and qpdf->password @@ -66,6 +71,12 @@ static void call_init_write(qpdf_data qpdf) qpdf->qpdf_writer = new QPDFWriter(*(qpdf->qpdf), qpdf->filename); } +static void call_init_write_memory(qpdf_data qpdf) +{ + qpdf->qpdf_writer = new QPDFWriter(*(qpdf->qpdf)); + qpdf->qpdf_writer->setOutputMemory(); +} + static void call_write(qpdf_data qpdf) { qpdf->qpdf_writer->write(); @@ -408,21 +419,71 @@ QPDF_BOOL qpdf_allow_modify_all(qpdf_data qpdf) return qpdf->qpdf->allowModifyAll(); } -QPDF_ERROR_CODE qpdf_init_write(qpdf_data qpdf, char const* filename) +static void qpdf_init_write_internal(qpdf_data qpdf) { - QPDF_ERROR_CODE status = QPDF_SUCCESS; if (qpdf->qpdf_writer) { QTC::TC("qpdf", "qpdf-c called qpdf_init_write multiple times"); delete qpdf->qpdf_writer; qpdf->qpdf_writer = 0; + if (qpdf->output_buffer) + { + delete qpdf->output_buffer; + qpdf->output_buffer = 0; + qpdf->write_memory = false; + qpdf->filename = 0; + } } +} + +QPDF_ERROR_CODE qpdf_init_write(qpdf_data qpdf, char const* filename) +{ + qpdf_init_write_internal(qpdf); qpdf->filename = filename; - status = trap_errors(qpdf, &call_init_write); + QPDF_ERROR_CODE status = trap_errors(qpdf, &call_init_write); QTC::TC("qpdf", "qpdf-c called qpdf_init_write", status); return status; } +QPDF_ERROR_CODE qpdf_init_write_memory(qpdf_data qpdf) +{ + qpdf_init_write_internal(qpdf); + QPDF_ERROR_CODE status = trap_errors(qpdf, &call_init_write_memory); + QTC::TC("qpdf", "qpdf-c called qpdf_init_write_memory"); + qpdf->write_memory = true; + return status; +} + +static void qpdf_get_buffer_internal(qpdf_data qpdf) +{ + if (qpdf->write_memory && (qpdf->output_buffer == 0)) + { + qpdf->output_buffer = qpdf->qpdf_writer->getBuffer(); + } +} + +unsigned long qpdf_get_buffer_length(qpdf_data qpdf) +{ + qpdf_get_buffer_internal(qpdf); + unsigned long result = 0L; + if (qpdf->output_buffer) + { + result = qpdf->output_buffer->getSize(); + } + return result; +} + +unsigned char const* qpdf_get_buffer(qpdf_data qpdf) +{ + unsigned char const* result = 0; + qpdf_get_buffer_internal(qpdf); + if (qpdf->output_buffer) + { + result = qpdf->output_buffer->getBuffer(); + } + return result; +} + void qpdf_set_object_stream_mode(qpdf_data qpdf, qpdf_object_stream_e mode) { QTC::TC("qpdf", "qpdf-c called qpdf_set_object_stream_mode"); diff --git a/libtests/buffer.cc b/libtests/buffer.cc index 9728009f..923be197 100644 --- a/libtests/buffer.cc +++ b/libtests/buffer.cc @@ -3,6 +3,7 @@ #include #include #include +#include typedef unsigned char* uc; diff --git a/manual/qpdf-manual.xml b/manual/qpdf-manual.xml index 0d743677..72e98f5c 100644 --- a/manual/qpdf-manual.xml +++ b/manual/qpdf-manual.xml @@ -2131,6 +2131,14 @@ print "\n"; /Info dictionary. + + + Add functions qpdf_init_write_memory, + qpdf_get_buffer_length, and + qpdf_get_buffer to the C API for writing + PDF files to a memory buffer instead of a file. + + diff --git a/qpdf/qpdf-ctest.c b/qpdf/qpdf-ctest.c index aa61b9a1..ffb1fff8 100644 --- a/qpdf/qpdf-ctest.c +++ b/qpdf/qpdf-ctest.c @@ -337,6 +337,10 @@ static void test16(char const* infile, char const* outfile, char const* outfile2) { + unsigned long buflen = 0L; + unsigned char const* buf = 0; + FILE* f = 0; + qpdf_read(qpdf, infile, password); print_info("/Author"); print_info("/Producer"); @@ -347,11 +351,22 @@ static void test16(char const* infile, print_info("/Author"); print_info("/Producer"); print_info("/Creator"); - qpdf_init_write(qpdf, outfile); + qpdf_init_write_memory(qpdf); qpdf_set_static_ID(qpdf, QPDF_TRUE); qpdf_set_static_aes_IV(qpdf, QPDF_TRUE); qpdf_set_stream_data_mode(qpdf, qpdf_s_uncompress); qpdf_write(qpdf); + f = fopen(outfile, "wb"); + if (f == NULL) + { + fprintf(stderr, "%s: unable to open %s: %s\n", + whoami, outfile, strerror(errno)); + exit(2); + } + buflen = qpdf_get_buffer_length(qpdf); + buf = qpdf_get_buffer(qpdf); + fwrite(buf, 1, buflen, f); + fclose(f); report_errors(); } diff --git a/qpdf/qpdf.testcov b/qpdf/qpdf.testcov index 520b7fbe..51a5f6aa 100644 --- a/qpdf/qpdf.testcov +++ b/qpdf/qpdf.testcov @@ -199,3 +199,4 @@ qpdf-c set_info_key to value 0 qpdf-c set_info_key to null 0 qpdf-c set-info-key use existing info 0 qpdf-c add info to trailer 0 +qpdf-c called qpdf_init_write_memory 0 diff --git a/qpdf/test_driver.cc b/qpdf/test_driver.cc index 89da9144..bc9bdfc9 100644 --- a/qpdf/test_driver.cc +++ b/qpdf/test_driver.cc @@ -76,7 +76,7 @@ void runtest(int n, char const* filename) fseek(f, 0, SEEK_END); size_t size = (size_t) ftell(f); fseek(f, 0, SEEK_SET); - file_buf = new char[size]; + file_buf = PointerHolder(true, new char[size]); char* buf_p = file_buf.getPointer(); size_t bytes_read = 0; size_t len = 0; -- cgit v1.2.3-54-g00ecf