In shippable code, favor smart pointers (fixes #235)

Use PointerHolder in several places where manually memory allocation
and deallocation were being used. This helps to protect against memory
leaks when exceptions are thrown in surprising places.

14 files changed, 73 insertions, 86 deletions

diff --git a/libqpdf/ClosedFileInputSource.cc b/libqpdf/ClosedFileInputSource.cc
index e9c9b3bd..f68a9a31 100644
--- a/libqpdf/ClosedFileInputSource.cc
+++ b/libqpdf/ClosedFileInputSource.cc
@@ -4,14 +4,12 @@
 ClosedFileInputSource::Members::Members(char const* filename) :
     filename(filename),
     offset(0),
-    fis(0),
     stay_open(false)
 {
 }
 
 ClosedFileInputSource::Members::~Members()
 {
-    delete fis;
 }
 
 ClosedFileInputSource::ClosedFileInputSource(char const* filename) :
@@ -26,7 +24,7 @@ ClosedFileInputSource::~ClosedFileInputSource()
 void
 ClosedFileInputSource::before()
 {
-    if (0 == this->m->fis)
+    if (0 == this->m->fis.getPointer())
     {
         this->m->fis = new FileInputSource();
         this->m->fis->setFilename(this->m->filename.c_str());
@@ -44,7 +42,6 @@ ClosedFileInputSource::after()
     {
         return;
     }
-    delete this->m->fis;
     this->m->fis = 0;
 }
 
@@ -84,7 +81,7 @@ void
 ClosedFileInputSource::rewind()
 {
     this->m->offset = 0;
-    if (this->m->fis)
+    if (this->m->fis.getPointer())
     {
         this->m->fis->rewind();
     }
@@ -112,7 +109,7 @@ void
 ClosedFileInputSource::stayOpen(bool val)
 {
     this->m->stay_open = val;
-    if ((! val) && this->m->fis)
+    if ((! val) && this->m->fis.getPointer())
     {
         after();
     }
diff --git a/libqpdf/Pl_AES_PDF.cc b/libqpdf/Pl_AES_PDF.cc
index c2c921e6..1099ab13 100644
--- a/libqpdf/Pl_AES_PDF.cc
+++ b/libqpdf/Pl_AES_PDF.cc
@@ -25,29 +25,31 @@ Pl_AES_PDF::Pl_AES_PDF(char const* identifier, Pipeline* next,
 {
     size_t keybits = 8 * key_bytes;
     assert(key_bytes == KEYLENGTH(keybits));
-    this->key = new unsigned char[key_bytes];
-    this->rk = new uint32_t[RKLENGTH(keybits)];
+    this->key = PointerHolder<unsigned char>(
+        true, new unsigned char[key_bytes]);
+    this->rk = PointerHolder<uint32_t>(
+        true, new uint32_t[RKLENGTH(keybits)]);
     size_t rk_bytes = RKLENGTH(keybits) * sizeof(uint32_t);
-    std::memcpy(this->key, key, key_bytes);
-    std::memset(this->rk, 0, rk_bytes);
+    std::memcpy(this->key.getPointer(), key, key_bytes);
+    std::memset(this->rk.getPointer(), 0, rk_bytes);
     std::memset(this->inbuf, 0, this->buf_size);
     std::memset(this->outbuf, 0, this->buf_size);
     std::memset(this->cbc_block, 0, this->buf_size);
     if (encrypt)
     {
-	this->nrounds = rijndaelSetupEncrypt(this->rk, this->key, keybits);
+	this->nrounds = rijndaelSetupEncrypt(
+            this->rk.getPointer(), this->key.getPointer(), keybits);
     }
     else
     {
-	this->nrounds = rijndaelSetupDecrypt(this->rk, this->key, keybits);
+	this->nrounds = rijndaelSetupDecrypt(
+            this->rk.getPointer(), this->key.getPointer(), keybits);
     }
     assert(this->nrounds == NROUNDS(keybits));
 }
 
 Pl_AES_PDF::~Pl_AES_PDF()
 {
-    delete [] this->key;
-    delete [] this->rk;
 }
 
 void
@@ -222,7 +224,8 @@ Pl_AES_PDF::flush(bool strip_padding)
 		this->inbuf[i] ^= this->cbc_block[i];
 	    }
 	}
-	rijndaelEncrypt(this->rk, this->nrounds, this->inbuf, this->outbuf);
+	rijndaelEncrypt(this->rk.getPointer(),
+                        this->nrounds, this->inbuf, this->outbuf);
 	if (this->cbc_mode)
 	{
 	    memcpy(this->cbc_block, this->outbuf, this->buf_size);
@@ -230,7 +233,8 @@ Pl_AES_PDF::flush(bool strip_padding)
     }
     else
     {
-	rijndaelDecrypt(this->rk, this->nrounds, this->inbuf, this->outbuf);
+	rijndaelDecrypt(this->rk.getPointer(),
+                        this->nrounds, this->inbuf, this->outbuf);
 	if (this->cbc_mode)
 	{
 	    for (unsigned int i = 0; i < this->buf_size; ++i)
diff --git a/libqpdf/Pl_Flate.cc b/libqpdf/Pl_Flate.cc
index c42d3c28..a782255b 100644
--- a/libqpdf/Pl_Flate.cc
+++ b/libqpdf/Pl_Flate.cc
@@ -13,7 +13,8 @@ Pl_Flate::Members::Members(size_t out_bufsize,
     initialized(false),
     zdata(0)
 {
-    this->outbuf = new unsigned char[out_bufsize];
+    this->outbuf = PointerHolder<unsigned char>(
+        true, new unsigned char[out_bufsize]);
     // Indirect through zdata to reach the z_stream so we don't have
     // to include zlib.h in Pl_Flate.hh.  This means people using
     // shared library versions of qpdf don't have to have zlib
@@ -34,15 +35,12 @@ Pl_Flate::Members::Members(size_t out_bufsize,
     zstream.opaque = 0;
     zstream.next_in = 0;
     zstream.avail_in = 0;
-    zstream.next_out = this->outbuf;
+    zstream.next_out = this->outbuf.getPointer();
     zstream.avail_out = QIntC::to_uint(out_bufsize);
 }
 
 Pl_Flate::Members::~Members()
 {
-    delete [] this->outbuf;
-    this->outbuf = 0;
-
     if (this->initialized)
     {
         z_stream& zstream = *(static_cast<z_stream*>(this->zdata));
@@ -74,7 +72,7 @@ Pl_Flate::~Pl_Flate()
 void
 Pl_Flate::write(unsigned char* data, size_t len)
 {
-    if (this->m->outbuf == 0)
+    if (this->m->outbuf.getPointer() == 0)
     {
 	throw std::logic_error(
 	    this->identifier +
@@ -186,8 +184,8 @@ Pl_Flate::handleData(unsigned char* data, size_t len, int flush)
                     QIntC::to_ulong(this->m->out_bufsize - zstream.avail_out);
 		if (ready > 0)
 		{
-		    this->getNext()->write(this->m->outbuf, ready);
-		    zstream.next_out = this->m->outbuf;
+		    this->getNext()->write(this->m->outbuf.getPointer(), ready);
+		    zstream.next_out = this->m->outbuf.getPointer();
 		    zstream.avail_out = QIntC::to_uint(this->m->out_bufsize);
 		}
 	    }
@@ -205,7 +203,7 @@ Pl_Flate::finish()
 {
     try
     {
-        if (this->m->outbuf)
+        if (this->m->outbuf.getPointer())
         {
             if (this->m->initialized)
             {
@@ -226,7 +224,6 @@ Pl_Flate::finish()
                 checkError("End", err);
             }
 
-            delete [] this->m->outbuf;
             this->m->outbuf = 0;
         }
     }
diff --git a/libqpdf/Pl_PNGFilter.cc b/libqpdf/Pl_PNGFilter.cc
index a39c17f9..6a3bc5b8 100644
--- a/libqpdf/Pl_PNGFilter.cc
+++ b/libqpdf/Pl_PNGFilter.cc
@@ -45,12 +45,14 @@ Pl_PNGFilter::Pl_PNGFilter(char const* identifier, Pipeline* next,
             "PNGFilter created with invalid columns value");
     }
     this->bytes_per_row = bpr & UINT_MAX;
-    this->buf1 = new unsigned char[this->bytes_per_row + 1];
-    this->buf2 = new unsigned char[this->bytes_per_row + 1];
-    memset(this->buf1, 0, this->bytes_per_row + 1);
-    memset(this->buf2, 0, this->bytes_per_row + 1);
-    this->cur_row = this->buf1;
-    this->prev_row = this->buf2;
+    this->buf1 = PointerHolder<unsigned char>(
+        true, new unsigned char[this->bytes_per_row + 1]);
+    this->buf2 = PointerHolder<unsigned char>(
+        true, new unsigned char[this->bytes_per_row + 1]);
+    memset(this->buf1.getPointer(), 0, this->bytes_per_row + 1);
+    memset(this->buf2.getPointer(), 0, this->bytes_per_row + 1);
+    this->cur_row = this->buf1.getPointer();
+    this->prev_row = this->buf2.getPointer();
 
     // number of bytes per incoming row
     this->incoming = (action == a_encode ?
@@ -60,8 +62,6 @@ Pl_PNGFilter::Pl_PNGFilter(char const* identifier, Pipeline* next,
 
 Pl_PNGFilter::~Pl_PNGFilter()
 {
-    delete [] buf1;
-    delete [] buf2;
 }
 
 void
@@ -81,7 +81,7 @@ Pl_PNGFilter::write(unsigned char* data, size_t len)
 	// Swap rows
 	unsigned char* t = this->prev_row;
 	this->prev_row = this->cur_row;
-	this->cur_row = t ? t : this->buf2;
+	this->cur_row = t ? t : this->buf2.getPointer();
 	memset(this->cur_row, 0, this->bytes_per_row + 1);
 	left = this->incoming;
 	this->pos = 0;
@@ -269,7 +269,7 @@ Pl_PNGFilter::finish()
 	processRow();
     }
     this->prev_row = 0;
-    this->cur_row = buf1;
+    this->cur_row = buf1.getPointer();
     this->pos = 0;
     memset(this->cur_row, 0, this->bytes_per_row + 1);
 
diff --git a/libqpdf/Pl_RC4.cc b/libqpdf/Pl_RC4.cc
index 407490ca..6c8fd3c6 100644
--- a/libqpdf/Pl_RC4.cc
+++ b/libqpdf/Pl_RC4.cc
@@ -8,19 +8,18 @@ Pl_RC4::Pl_RC4(char const* identifier, Pipeline* next,
     out_bufsize(out_bufsize),
     rc4(key_data, key_len)
 {
-    this->outbuf = new unsigned char[out_bufsize];
+    this->outbuf = PointerHolder<unsigned char>(
+        true, new unsigned char[out_bufsize]);
 }
 
 Pl_RC4::~Pl_RC4()
 {
-    delete [] this->outbuf;
-    this->outbuf = 0;
 }
 
 void
 Pl_RC4::write(unsigned char* data, size_t len)
 {
-    if (this->outbuf == 0)
+    if (this->outbuf.getPointer() == 0)
     {
 	throw std::logic_error(
 	    this->identifier +
@@ -35,16 +34,15 @@ Pl_RC4::write(unsigned char* data, size_t len)
 	size_t bytes =
             (bytes_left < this->out_bufsize ? bytes_left : out_bufsize);
 	bytes_left -= bytes;
-	rc4.process(p, bytes, outbuf);
+	rc4.process(p, bytes, outbuf.getPointer());
 	p += bytes;
-	getNext()->write(outbuf, bytes);
+	getNext()->write(outbuf.getPointer(), bytes);
     }
 }
 
 void
 Pl_RC4::finish()
 {
-    delete [] this->outbuf;
     this->outbuf = 0;
     this->getNext()->finish();
 }
diff --git a/libqpdf/Pl_TIFFPredictor.cc b/libqpdf/Pl_TIFFPredictor.cc
index 42cfbb1a..30487511 100644
--- a/libqpdf/Pl_TIFFPredictor.cc
+++ b/libqpdf/Pl_TIFFPredictor.cc
@@ -16,7 +16,6 @@ Pl_TIFFPredictor::Pl_TIFFPredictor(char const* identifier, Pipeline* next,
     columns(columns),
     samples_per_pixel(samples_per_pixel),
     bits_per_sample(bits_per_sample),
-    cur_row(0),
     pos(0)
 {
     if (samples_per_pixel < 1)
@@ -38,13 +37,13 @@ Pl_TIFFPredictor::Pl_TIFFPredictor(char const* identifier, Pipeline* next,
             "TIFFPredictor created with invalid columns value");
     }
     this->bytes_per_row = bpr & UINT_MAX;
-    this->cur_row = new unsigned char[this->bytes_per_row];
-    memset(this->cur_row, 0, this->bytes_per_row);
+    this->cur_row = PointerHolder<unsigned char>(
+        true, new unsigned char[this->bytes_per_row]);
+    memset(this->cur_row.getPointer(), 0, this->bytes_per_row);
 }
 
 Pl_TIFFPredictor::~Pl_TIFFPredictor()
 {
-    delete [] cur_row;
 }
 
 void
@@ -55,20 +54,20 @@ Pl_TIFFPredictor::write(unsigned char* data, size_t len)
     while (len >= left)
     {
 	// finish off current row
-	memcpy(this->cur_row + this->pos, data + offset, left);
+	memcpy(this->cur_row.getPointer() + this->pos, data + offset, left);
 	offset += left;
 	len -= left;
 
 	processRow();
 
 	// Prepare for next row
-	memset(this->cur_row, 0, this->bytes_per_row);
+	memset(this->cur_row.getPointer(), 0, this->bytes_per_row);
 	left = this->bytes_per_row;
 	this->pos = 0;
     }
     if (len)
     {
-	memcpy(this->cur_row + this->pos, data + offset, len);
+	memcpy(this->cur_row.getPointer() + this->pos, data + offset, len);
     }
     this->pos += len;
 }
@@ -79,7 +78,7 @@ Pl_TIFFPredictor::processRow()
     QTC::TC("libtests", "Pl_TIFFPredictor processRow",
             (action == a_decode ? 0 : 1));
     BitWriter bw(this->getNext());
-    BitStream in(this->cur_row, this->bytes_per_row);
+    BitStream in(this->cur_row.getPointer(), this->bytes_per_row);
     std::vector<long long> prev;
     for (unsigned int i = 0; i < this->samples_per_pixel; ++i)
     {
@@ -118,6 +117,6 @@ Pl_TIFFPredictor::finish()
 	processRow();
     }
     this->pos = 0;
-    memset(this->cur_row, 0, this->bytes_per_row);
+    memset(this->cur_row.getPointer(), 0, this->bytes_per_row);
     getNext()->finish();
 }
diff --git a/libqpdf/QPDFWriter.cc b/libqpdf/QPDFWriter.cc
index 7702dc34..41d5ab30 100644
--- a/libqpdf/QPDFWriter.cc
+++ b/libqpdf/QPDFWriter.cc
@@ -1835,21 +1835,21 @@ QPDFWriter::unparseObject(QPDFObjectHandle object, int level,
                     this->m->cur_data_key.length());
 		pl.write(QUtil::unsigned_char_pointer(val), val.length());
 		pl.finish();
-		Buffer* buf = bufpl.getBuffer();
+		PointerHolder<Buffer> buf = bufpl.getBuffer();
 		val = QPDF_String(
 		    std::string(reinterpret_cast<char*>(buf->getBuffer()),
 				buf->getSize())).unparse(true);
-		delete buf;
 	    }
 	    else
 	    {
-		char* tmp = QUtil::copy_string(val);
+		PointerHolder<char> tmp_ph =
+                    PointerHolder<char>(true, QUtil::copy_string(val));
+                char* tmp = tmp_ph.getPointer();
 		size_t vlen = val.length();
 		RC4 rc4(QUtil::unsigned_char_pointer(this->m->cur_data_key),
 			QIntC::to_int(this->m->cur_data_key.length()));
 		rc4.process(QUtil::unsigned_char_pointer(tmp), vlen);
 		val = QPDF_String(std::string(tmp, vlen)).unparse();
-		delete [] tmp;
 	    }
 	}
 	else
diff --git a/libqpdf/QPDF_encryption.cc b/libqpdf/QPDF_encryption.cc
index d72d6fb7..b5f16e0c 100644
--- a/libqpdf/QPDF_encryption.cc
+++ b/libqpdf/QPDF_encryption.cc
@@ -204,7 +204,9 @@ iterate_rc4(unsigned char* data, size_t data_len,
 	    unsigned char* okey, int key_len,
 	    int iterations, bool reverse)
 {
-    unsigned char* key = new unsigned char[QIntC::to_size(key_len)];
+    PointerHolder<unsigned char> key_ph = PointerHolder<unsigned char>(
+        true, new unsigned char[QIntC::to_size(key_len)]);
+    unsigned char* key = key_ph.getPointer();
     for (int i = 0; i < iterations; ++i)
     {
 	int const xor_value = (reverse ? iterations - 1 - i : i);
@@ -215,7 +217,6 @@ iterate_rc4(unsigned char* data, size_t data_len,
 	RC4 rc4(key, QIntC::to_int(key_len));
 	rc4.process(data, data_len);
     }
-    delete [] key;
 }
 
 static std::string
diff --git a/libqpdf/QUtil.cc b/libqpdf/QUtil.cc
index 151832fb..a3bf1744 100644
--- a/libqpdf/QUtil.cc
+++ b/libqpdf/QUtil.cc
@@ -670,10 +670,9 @@ QUtil::get_env(std::string const& var, std::string* value)
 
     if (value)
     {
-	char* t = new char[len + 1];
-        ::GetEnvironmentVariable(var.c_str(), t, len);
-	*value = t;
-	delete [] t;
+	PointerHolder<char> t = PointerHolder<char>(true, new char[len + 1]);
+        ::GetEnvironmentVariable(var.c_str(), t.getPointer(), len);
+	*value = t.getPointer();
     }
 
     return true;
diff --git a/libqpdf/qpdf-c.cc b/libqpdf/qpdf-c.cc
index f42b0e16..fa9045ea 100644
--- a/libqpdf/qpdf-c.cc
+++ b/libqpdf/qpdf-c.cc
@@ -22,8 +22,8 @@ struct _qpdf_data
     _qpdf_data();
     ~_qpdf_data();
 
-    QPDF* qpdf;
-    QPDFWriter* qpdf_writer;
+    PointerHolder<QPDF> qpdf;
+    PointerHolder<QPDFWriter> qpdf_writer;
 
     PointerHolder<QPDFExc> error;
     _qpdf_error tmp_error;
@@ -36,22 +36,16 @@ struct _qpdf_data
     unsigned long long size;
     char const* password;
     bool write_memory;
-    Buffer* output_buffer;
+    PointerHolder<Buffer> output_buffer;
 };
 
 _qpdf_data::_qpdf_data() :
-    qpdf(0),
-    qpdf_writer(0),
-    write_memory(false),
-    output_buffer(0)
+    write_memory(false)
 {
 }
 
 _qpdf_data::~_qpdf_data()
 {
-    delete qpdf_writer;
-    delete qpdf;
-    delete output_buffer;
 }
 
 // must set qpdf->filename and qpdf->password
@@ -451,14 +445,12 @@ QPDF_BOOL qpdf_allow_modify_all(qpdf_data qpdf)
 
 static void qpdf_init_write_internal(qpdf_data qpdf)
 {
-    if (qpdf->qpdf_writer)
+    if (qpdf->qpdf_writer.getPointer())
     {
 	QTC::TC("qpdf", "qpdf-c called qpdf_init_write multiple times");
-	delete qpdf->qpdf_writer;
 	qpdf->qpdf_writer = 0;
-	if (qpdf->output_buffer)
+	if (qpdf->output_buffer.getPointer())
 	{
-	    delete qpdf->output_buffer;
 	    qpdf->output_buffer = 0;
 	    qpdf->write_memory = false;
 	    qpdf->filename = 0;
@@ -496,7 +488,7 @@ size_t qpdf_get_buffer_length(qpdf_data qpdf)
 {
     qpdf_get_buffer_internal(qpdf);
     size_t result = 0;
-    if (qpdf->output_buffer)
+    if (qpdf->output_buffer.getPointer())
     {
 	result = qpdf->output_buffer->getSize();
     }
@@ -507,7 +499,7 @@ unsigned char const* qpdf_get_buffer(qpdf_data qpdf)
 {
     unsigned char const* result = 0;
     qpdf_get_buffer_internal(qpdf);
-    if (qpdf->output_buffer)
+    if (qpdf->output_buffer.getPointer())
     {
 	result = qpdf->output_buffer->getBuffer();
     }
diff --git a/libqpdf/qpdf/Pl_AES_PDF.hh b/libqpdf/qpdf/Pl_AES_PDF.hh
index 4aaf68bb..31cc28f9 100644
--- a/libqpdf/qpdf/Pl_AES_PDF.hh
+++ b/libqpdf/qpdf/Pl_AES_PDF.hh
@@ -55,8 +55,8 @@ class Pl_AES_PDF: public Pipeline
     bool cbc_mode;
     bool first;
     size_t offset;              // offset into memory buffer
-    unsigned char* key;
-    uint32_t* rk;
+    PointerHolder<unsigned char> key;
+    PointerHolder<uint32_t> rk;
     unsigned char inbuf[buf_size];
     unsigned char outbuf[buf_size];
     unsigned char cbc_block[buf_size];
diff --git a/libqpdf/qpdf/Pl_PNGFilter.hh b/libqpdf/qpdf/Pl_PNGFilter.hh
index f6745319..3530a73a 100644
--- a/libqpdf/qpdf/Pl_PNGFilter.hh
+++ b/libqpdf/qpdf/Pl_PNGFilter.hh
@@ -41,10 +41,10 @@ class Pl_PNGFilter: public Pipeline
     action_e action;
     unsigned int bytes_per_row;
     unsigned int bytes_per_pixel;
-    unsigned char* cur_row;
-    unsigned char* prev_row;
-    unsigned char* buf1;
-    unsigned char* buf2;
+    unsigned char* cur_row;     // points to buf1 or buf2
+    unsigned char* prev_row;    // points to buf1 or buf2
+    PointerHolder<unsigned char> buf1;
+    PointerHolder<unsigned char> buf2;
     size_t pos;
     size_t incoming;
 };
diff --git a/libqpdf/qpdf/Pl_RC4.hh b/libqpdf/qpdf/Pl_RC4.hh
index 9885f0ad..250ab2b8 100644
--- a/libqpdf/qpdf/Pl_RC4.hh
+++ b/libqpdf/qpdf/Pl_RC4.hh
@@ -24,7 +24,7 @@ class Pl_RC4: public Pipeline
     virtual void finish();
 
   private:
-    unsigned char* outbuf;
+    PointerHolder<unsigned char> outbuf;
     size_t out_bufsize;
     RC4 rc4;
 };
diff --git a/libqpdf/qpdf/Pl_TIFFPredictor.hh b/libqpdf/qpdf/Pl_TIFFPredictor.hh
index b4391f8e..484f4df4 100644
--- a/libqpdf/qpdf/Pl_TIFFPredictor.hh
+++ b/libqpdf/qpdf/Pl_TIFFPredictor.hh
@@ -32,7 +32,7 @@ class Pl_TIFFPredictor: public Pipeline
     unsigned int bytes_per_row;
     unsigned int samples_per_pixel;
     unsigned int bits_per_sample;
-    unsigned char* cur_row;
+    PointerHolder<unsigned char> cur_row;
     size_t pos;
 };