[poppler] Branch 'better_object' - poppler/Catalog.cc poppler/Catalog.h poppler/Dict.h poppler/Page.cc poppler/Page.h poppler/PDFDoc.cc poppler/PDFDoc.h utils/pdfunite.cc

Albert Astals Cid aacid at kemper.freedesktop.org
Fri May 12 21:38:41 UTC 2017


 poppler/Catalog.cc |   40 +++++++++++-----------------------------
 poppler/Catalog.h  |    2 +-
 poppler/Dict.h     |    9 +++++----
 poppler/PDFDoc.cc  |   37 ++++++++++++++++---------------------
 poppler/PDFDoc.h   |    7 +++----
 poppler/Page.cc    |   17 ++++++++---------
 poppler/Page.h     |    2 +-
 utils/pdfunite.cc  |   12 +++++-------
 8 files changed, 50 insertions(+), 76 deletions(-)

New commits:
commit 47aa71cf39764135f3e3a39cbfb8efc50de1ac28
Author: Albert Astals Cid <aacid at kde.org>
Date:   Fri May 12 23:38:26 2017 +0200

    Make Dict incRef/decRef private

diff --git a/poppler/Catalog.cc b/poppler/Catalog.cc
index 8ffff9dc..6c820c66 100644
--- a/poppler/Catalog.cc
+++ b/poppler/Catalog.cc
@@ -148,15 +148,7 @@ Catalog::~Catalog() {
     delete attrsList;
   }
   delete pagesRefList;
-  if (pagesList) {
-    std::vector<Dict *>::iterator it;
-    for (it = pagesList->begin() ; it != pagesList->end(); ++it ) {
-      if (!(*it)->decRef()) {
-         delete *it;
-      }
-    }
-    delete pagesList;
-  }
+  delete pagesList;
   if (pages) {
     for (int i = 0; i < pagesSize; ++i) {
       if (pages[i]) {
@@ -238,8 +230,6 @@ Ref *Catalog::getPageRef(int i)
 
 GBool Catalog::cachePageTree(int page)
 {
-  Dict *pagesDict;
-
   if (pagesList == NULL) {
 
     Ref pagesRef;
@@ -264,10 +254,7 @@ GBool Catalog::cachePageTree(int page)
     Object obj = catDict.dictLookup("Pages");
     // This should really be isDict("Pages"), but I've seen at least one
     // PDF file where the /Type entry is missing.
-    if (obj.isDict()) {
-      obj.getDict()->incRef();
-      pagesDict = obj.getDict();
-    } else {
+    if (!obj.isDict()) {
       error(errSyntaxError, -1, "Top-level pages object is wrong type ({0:s})", obj.getTypeName());
       return gFalse;
     }
@@ -277,7 +264,6 @@ GBool Catalog::cachePageTree(int page)
     pageRefs = (Ref *)gmallocn_checkoverflow(pagesSize, sizeof(Ref));
     if (pages == NULL || pageRefs == NULL ) {
       error(errSyntaxError, -1, "Cannot allocate page cache");
-      pagesDict->decRef();
       pagesSize = 0;
       return gFalse;
     }
@@ -287,12 +273,12 @@ GBool Catalog::cachePageTree(int page)
       pageRefs[i].gen = -1;
     }
 
-    pagesList = new std::vector<Dict *>();
-    pagesList->push_back(pagesDict);
+    attrsList = new std::vector<PageAttrs *>();
+    attrsList->push_back(new PageAttrs(NULL, obj.getDict()));
+    pagesList = new std::vector<Object>();
+    pagesList->push_back(std::move(obj));
     pagesRefList = new std::vector<Ref>();
     pagesRefList->push_back(pagesRef);
-    attrsList = new std::vector<PageAttrs *>();
-    attrsList->push_back(new PageAttrs(NULL, pagesDict));
     kidsIdxList = new std::vector<int>();
     kidsIdxList->push_back(0);
     lastCachedPage = 0;
@@ -305,8 +291,8 @@ GBool Catalog::cachePageTree(int page)
 
     if (pagesList->empty()) return gFalse;
 
-    pagesDict = pagesList->back();
-    Object kids = pagesDict->lookup("Kids");
+    Object pagesDict = pagesList->back().copy();
+    Object kids = pagesDict.dictLookup("Kids");
     if (!kids.isArray()) {
       error(errSyntaxError, -1, "Kids object (page {0:d}) is wrong type ({1:s})",
             lastCachedPage+1, kids.getTypeName());
@@ -315,9 +301,6 @@ GBool Catalog::cachePageTree(int page)
 
     int kidsIdx = kidsIdxList->back();
     if (kidsIdx >= kids.arrayGetLength()) {
-       if (!pagesList->back()->decRef()) {
-         delete pagesList->back();
-       }
        pagesList->pop_back();
        pagesRefList->pop_back();
        delete attrsList->back();
@@ -350,7 +333,7 @@ GBool Catalog::cachePageTree(int page)
     Object kid = kids.arrayGet(kidsIdx);
     if (kid.isDict("Page") || (kid.isDict() && !kid.getDict()->hasKey("Kids"))) {
       PageAttrs *attrs = new PageAttrs(attrsList->back(), kid.getDict());
-      Page *p = new Page(doc, lastCachedPage+1, kid.getDict(),
+      Page *p = new Page(doc, lastCachedPage+1, &kid,
                      kidRef.getRef(), attrs, form);
       if (!p->isOk()) {
         error(errSyntaxError, -1, "Failed to create page (page {0:d})", lastCachedPage+1);
@@ -375,8 +358,7 @@ GBool Catalog::cachePageTree(int page)
     } else if (kid.isDict()) {
       attrsList->push_back(new PageAttrs(attrsList->back(), kid.getDict()));
       pagesRefList->push_back(kidRef.getRef());
-      kid.getDict()->incRef();
-      pagesList->push_back(kid.getDict());
+      pagesList->push_back(std::move(kid));
       kidsIdxList->push_back(0);
     } else {
       error(errSyntaxError, -1, "Kid object (page {0:d}) is wrong type ({1:s})",
@@ -792,7 +774,7 @@ int Catalog::getNumPages()
 	Dict *pageDict = pagesDict.getDict();
 	if (pageRootRef.isRef()) {
 	  const Ref pageRef = pageRootRef.getRef();
-	  Page *p = new Page(doc, 1, pageDict, pageRef, new PageAttrs(NULL, pageDict), form);
+	  Page *p = new Page(doc, 1, &pagesDict, pageRef, new PageAttrs(NULL, pageDict), form);
 	  if (p->isOk()) {
 	    pages = (Page **)gmallocn(1, sizeof(Page *));
 	    pageRefs = (Ref *)gmallocn(1, sizeof(Ref));
diff --git a/poppler/Catalog.h b/poppler/Catalog.h
index eb324b78..c8b501a1 100644
--- a/poppler/Catalog.h
+++ b/poppler/Catalog.h
@@ -248,7 +248,7 @@ private:
   Page **pages;			// array of pages
   Ref *pageRefs;		// object ID for each page
   int lastCachedPage;
-  std::vector<Dict *> *pagesList;
+  std::vector<Object> *pagesList;
   std::vector<Ref> *pagesRefList;
   std::vector<PageAttrs *> *attrsList;
   std::vector<int> *kidsIdxList;
diff --git a/poppler/Dict.h b/poppler/Dict.h
index 0e65bfed..b775a574 100644
--- a/poppler/Dict.h
+++ b/poppler/Dict.h
@@ -56,10 +56,6 @@ public:
   // Destructor.
   ~Dict();
 
-  // Reference counting.
-  int incRef();
-  int decRef();
-
   // Get number of entries.
   int getLength() { return length; }
 
@@ -97,6 +93,11 @@ public:
   GBool hasKey(const char *key);
 
 private:
+  friend class Object; // for incRef/decRef
+
+  // Reference counting.
+  int incRef();
+  int decRef();
 
   GBool sorted;
   XRef *xref;			// the xref table for this PDF file
diff --git a/poppler/PDFDoc.cc b/poppler/PDFDoc.cc
index 16981b93..206b7e42 100644
--- a/poppler/PDFDoc.cc
+++ b/poppler/PDFDoc.cc
@@ -861,11 +861,10 @@ int PDFDoc::savePageAs(GooString *name, int pageNo)
   Ref ref;
   ref.num = rootNum;
   ref.gen = 0;
-  Dict *trailerDict = createTrailerDict(rootNum + 3, gFalse, 0, &ref, getXRef(),
+  Object trailerDict = createTrailerDict(rootNum + 3, gFalse, 0, &ref, getXRef(),
                                         name->getCString(), uxrefOffset);
-  writeXRefTableTrailer(trailerDict, yRef, gFalse /* do not write unnecessary entries */,
+  writeXRefTableTrailer(std::move(trailerDict), yRef, gFalse /* do not write unnecessary entries */,
                         uxrefOffset, outStr, getXRef());
-  delete trailerDict;
 
   outStr->close();
   fclose(f);
@@ -1004,14 +1003,13 @@ void PDFDoc::saveIncrementalUpdate (OutStream* outStr)
     uxref->add(uxrefStreamRef.num, uxrefStreamRef.gen, uxrefOffset, gTrue);
   }
 
-  Dict *trailerDict = createTrailerDict(numobjects, gTrue, getStartXRef(), &rootRef, getXRef(), fileNameA, uxrefOffset);
+  Object trailerDict = createTrailerDict(numobjects, gTrue, getStartXRef(), &rootRef, getXRef(), fileNameA, uxrefOffset);
   if (xRefStream) {
-    writeXRefStreamTrailer(trailerDict, uxref, &uxrefStreamRef, uxrefOffset, outStr, getXRef());
+    writeXRefStreamTrailer(std::move(trailerDict), uxref, &uxrefStreamRef, uxrefOffset, outStr, getXRef());
   } else {
-    writeXRefTableTrailer(trailerDict, uxref, gFalse, uxrefOffset, outStr, getXRef());
+    writeXRefTableTrailer(std::move(trailerDict), uxref, gFalse, uxrefOffset, outStr, getXRef());
   }
 
-  delete trailerDict;
   delete uxref;
 }
 
@@ -1340,7 +1338,7 @@ void PDFDoc::writeObjectFooter (OutStream* outStr)
   outStr->printf("endobj\r\n");
 }
 
-Dict *PDFDoc::createTrailerDict(int uxrefSize, GBool incrUpdate, Goffset startxRef,
+Object PDFDoc::createTrailerDict(int uxrefSize, GBool incrUpdate, Goffset startxRef,
                                 Ref *root, XRef *xRef, const char *fileName, Goffset fileSize)
 {
   Dict *trailerDict = new Dict(xRef);
@@ -1421,29 +1419,28 @@ Dict *PDFDoc::createTrailerDict(int uxrefSize, GBool incrUpdate, Goffset startxR
     }
   }
 
-  return trailerDict;
+  return Object(trailerDict);
 }
 
-void PDFDoc::writeXRefTableTrailer(Dict *trailerDict, XRef *uxref, GBool writeAllEntries, Goffset uxrefOffset, OutStream* outStr, XRef *xRef)
+void PDFDoc::writeXRefTableTrailer(Object &&trailerDict, XRef *uxref, GBool writeAllEntries, Goffset uxrefOffset, OutStream* outStr, XRef *xRef)
 {
   uxref->writeTableToFile( outStr, writeAllEntries );
   outStr->printf( "trailer\r\n");
-  writeDictionnary(trailerDict, outStr, xRef, 0, NULL, cryptRC4, 0, 0, 0);
+  writeDictionnary(trailerDict.getDict(), outStr, xRef, 0, NULL, cryptRC4, 0, 0, 0);
   outStr->printf( "\r\nstartxref\r\n");
   outStr->printf( "%lli\r\n", uxrefOffset);
   outStr->printf( "%%%%EOF\r\n");
 }
 
-void PDFDoc::writeXRefStreamTrailer (Dict *trailerDict, XRef *uxref, Ref *uxrefStreamRef, Goffset uxrefOffset, OutStream* outStr, XRef *xRef)
+void PDFDoc::writeXRefStreamTrailer (Object &&trailerDict, XRef *uxref, Ref *uxrefStreamRef, Goffset uxrefOffset, OutStream* outStr, XRef *xRef)
 {
   GooString stmData;
 
   // Fill stmData and some trailerDict fields
-  uxref->writeStreamToBuffer(&stmData, trailerDict, xRef);
+  uxref->writeStreamToBuffer(&stmData, trailerDict.getDict(), xRef);
 
   // Create XRef stream object and write it
-  trailerDict->incRef();
-  MemStream *mStream = new MemStream( stmData.getCString(), 0, stmData.getLength(), Object(trailerDict) );
+  MemStream *mStream = new MemStream( stmData.getCString(), 0, stmData.getLength(), std::move(trailerDict) );
   writeObjectHeader(uxrefStreamRef, outStr);
   Object obj1(static_cast<Stream*>(mStream));
   writeObject(&obj1, outStr, xRef, 0, NULL, cryptRC4, 0, 0, 0);
@@ -1469,10 +1466,9 @@ void PDFDoc::writeXRefTableTrailer(Goffset uxrefOffset, XRef *uxref, GBool write
   Ref ref;
   ref.num = getXRef()->getRootNum();
   ref.gen = getXRef()->getRootGen();
-  Dict * trailerDict = createTrailerDict(uxrefSize, incrUpdate, getStartXRef(), &ref,
+  Object trailerDict = createTrailerDict(uxrefSize, incrUpdate, getStartXRef(), &ref,
                                          getXRef(), fileNameA, fileSize);
-  writeXRefTableTrailer(trailerDict, uxref, writeAllEntries, uxrefOffset, outStr, getXRef());
-  delete trailerDict;
+  writeXRefTableTrailer(std::move(trailerDict), uxref, writeAllEntries, uxrefOffset, outStr, getXRef());
 }
 
 void PDFDoc::writeHeader(OutStream *outStr, int major, int minor)
@@ -1890,7 +1886,6 @@ int PDFDoc::getNumPages()
 Page *PDFDoc::parsePage(int page)
 {
   Ref pageRef;
-  Dict *pageDict;
 
   pageRef.num = getHints()->getPageObjectNum(page);
   if (!pageRef.num) {
@@ -1910,9 +1905,9 @@ Page *PDFDoc::parsePage(int page)
     error(errSyntaxWarning, -1, "Object ({0:d} {1:d}) is not a pageDict", pageRef.num, pageRef.gen);
     return NULL;
   }
-  pageDict = obj.getDict();
+  Dict *pageDict = obj.getDict();
 
-  return new Page(this, page, pageDict, pageRef,
+  return new Page(this, page, &obj, pageRef,
                new PageAttrs(NULL, pageDict), catalog->getForm());
 }
 
diff --git a/poppler/PDFDoc.h b/poppler/PDFDoc.h
index 2c703b14..6a83c7c5 100644
--- a/poppler/PDFDoc.h
+++ b/poppler/PDFDoc.h
@@ -301,12 +301,11 @@ public:
                            CryptAlgorithm encAlgorithm, int keyLength, int objNum, int objGen);
   static void writeHeader(OutStream *outStr, int major, int minor);
 
-  // Ownership goes to the caller
-  static Dict *createTrailerDict (int uxrefSize, GBool incrUpdate, Goffset startxRef,
+  static Object createTrailerDict (int uxrefSize, GBool incrUpdate, Goffset startxRef,
                                   Ref *root, XRef *xRef, const char *fileName, Goffset fileSize);
-  static void writeXRefTableTrailer (Dict *trailerDict, XRef *uxref, GBool writeAllEntries,
+  static void writeXRefTableTrailer (Object &&trailerDict, XRef *uxref, GBool writeAllEntries,
                                      Goffset uxrefOffset, OutStream* outStr, XRef *xRef);
-  static void writeXRefStreamTrailer (Dict *trailerDict, XRef *uxref, Ref *uxrefStreamRef,
+  static void writeXRefStreamTrailer (Object &&trailerDict, XRef *uxref, Ref *uxrefStreamRef,
                                       Goffset uxrefOffset, OutStream* outStr, XRef *xRef);
 
 private:
diff --git a/poppler/Page.cc b/poppler/Page.cc
index b8002023..197e2bef 100644
--- a/poppler/Page.cc
+++ b/poppler/Page.cc
@@ -248,7 +248,7 @@ GBool PageAttrs::readBox(Dict *dict, const char *key, PDFRectangle *box) {
 // Page
 //------------------------------------------------------------------------
 
-Page::Page(PDFDoc *docA, int numA, Dict *pageDict, Ref pageRefA, PageAttrs *attrsA, Form *form) {
+Page::Page(PDFDoc *docA, int numA, Object *pageDict, Ref pageRefA, PageAttrs *attrsA, Form *form) {
 #if MULTITHREADED
   gInitMutex(&mutex);
 #endif
@@ -259,8 +259,7 @@ Page::Page(PDFDoc *docA, int numA, Dict *pageDict, Ref pageRefA, PageAttrs *attr
   duration = -1;
   annots = NULL;
 
-  pageDict->incRef();
-  pageObj = Object(pageDict);
+  pageObj = pageDict->copy();
   pageRef = pageRefA;
 
   // get attributes
@@ -268,7 +267,7 @@ Page::Page(PDFDoc *docA, int numA, Dict *pageDict, Ref pageRefA, PageAttrs *attr
   attrs->clipBoxes();
 
   // transtion
-  trans = pageDict->lookupNF("Trans");
+  trans = pageDict->dictLookupNF("Trans");
   if (!(trans.isRef() || trans.isDict() || trans.isNull())) {
     error(errSyntaxError, -1, "Page transition object (page {0:d}) is wrong type ({1:s})",
 	  num, trans.getTypeName());
@@ -276,7 +275,7 @@ Page::Page(PDFDoc *docA, int numA, Dict *pageDict, Ref pageRefA, PageAttrs *attr
   }
 
   // duration
-  Object tmp = pageDict->lookupNF("Dur");
+  Object tmp = pageDict->dictLookupNF("Dur");
   if (!(tmp.isNum() || tmp.isNull())) {
     error(errSyntaxError, -1, "Page duration object (page {0:d}) is wrong type ({1:s})",
 	  num, tmp.getTypeName());
@@ -285,7 +284,7 @@ Page::Page(PDFDoc *docA, int numA, Dict *pageDict, Ref pageRefA, PageAttrs *attr
   }
 
   // annotations
-  annotsObj = pageDict->lookupNF("Annots");
+  annotsObj = pageDict->dictLookupNF("Annots");
   if (!(annotsObj.isRef() || annotsObj.isArray() || annotsObj.isNull())) {
     error(errSyntaxError, -1, "Page annotations object (page {0:d}) is wrong type ({1:s})",
 	  num, annotsObj.getTypeName());
@@ -293,7 +292,7 @@ Page::Page(PDFDoc *docA, int numA, Dict *pageDict, Ref pageRefA, PageAttrs *attr
   }
 
   // contents
-  contents = pageDict->lookupNF("Contents");
+  contents = pageDict->dictLookupNF("Contents");
   if (!(contents.isRef() || contents.isArray() ||
 	contents.isNull())) {
     error(errSyntaxError, -1, "Page contents object (page {0:d}) is wrong type ({1:s})",
@@ -302,7 +301,7 @@ Page::Page(PDFDoc *docA, int numA, Dict *pageDict, Ref pageRefA, PageAttrs *attr
   }
 
   // thumb
-  thumb = pageDict->lookupNF("Thumb");
+  thumb = pageDict->dictLookupNF("Thumb");
   if (!(thumb.isStream() || thumb.isNull() || thumb.isRef())) {
       error(errSyntaxError, -1, "Page thumb object (page {0:d}) is wrong type ({1:s})",
             num, thumb.getTypeName());
@@ -310,7 +309,7 @@ Page::Page(PDFDoc *docA, int numA, Dict *pageDict, Ref pageRefA, PageAttrs *attr
   }
 
   // actions
-  actions = pageDict->lookupNF("AA");
+  actions = pageDict->dictLookupNF("AA");
   if (!(actions.isDict() || actions.isNull())) {
       error(errSyntaxError, -1, "Page additional action object (page {0:d}) is wrong type ({1:s})",
             num, actions.getTypeName());
diff --git a/poppler/Page.h b/poppler/Page.h
index e548c867..2a597131 100644
--- a/poppler/Page.h
+++ b/poppler/Page.h
@@ -143,7 +143,7 @@ class Page {
 public:
 
   // Constructor.
-  Page(PDFDoc *docA, int numA, Dict *pageDict, Ref pageRefA, PageAttrs *attrsA, Form *form);
+  Page(PDFDoc *docA, int numA, Object *pageDict, Ref pageRefA, PageAttrs *attrsA, Form *form);
 
   // Destructor.
   ~Page();
diff --git a/utils/pdfunite.cc b/utils/pdfunite.cc
index 55615e01..dd563857 100644
--- a/utils/pdfunite.cc
+++ b/utils/pdfunite.cc
@@ -283,10 +283,9 @@ int main (int argc, char *argv[])
       Ref *refPage = docs[i]->getCatalog()->getPageRef(j);
       Object page = docs[i]->getXRef()->fetch(refPage->num, refPage->gen);
       Dict *pageDict = page.getDict();
-      Dict *resDict = docs[i]->getCatalog()->getPage(j)->getResourceDict();
-      if (resDict) {
-        resDict->incRef();
-        pageDict->set("Resources", Object(resDict));
+      Object *resDict = docs[i]->getCatalog()->getPage(j)->getResourceDictObject();
+      if (resDict->isDict()) {
+        pageDict->set("Resources", resDict->copy());
       }
       pages.push_back(std::move(page));
       offsets.push_back(numOffset);
@@ -382,11 +381,10 @@ int main (int argc, char *argv[])
   Ref ref;
   ref.num = rootNum;
   ref.gen = 0;
-  Dict *trailerDict = PDFDoc::createTrailerDict(objectsCount, gFalse, 0, &ref, yRef,
+  Object trailerDict = PDFDoc::createTrailerDict(objectsCount, gFalse, 0, &ref, yRef,
                                                 fileName, outStr->getPos());
-  PDFDoc::writeXRefTableTrailer(trailerDict, yRef, gTrue, // write all entries according to ISO 32000-1, 7.5.4 Cross-Reference Table: "For a file that has never been incrementally updated, the cross-reference section shall contain only one subsection, whose object numbering begins at 0."
+  PDFDoc::writeXRefTableTrailer(std::move(trailerDict), yRef, gTrue, // write all entries according to ISO 32000-1, 7.5.4 Cross-Reference Table: "For a file that has never been incrementally updated, the cross-reference section shall contain only one subsection, whose object numbering begins at 0."
                                 uxrefOffset, outStr, yRef);
-  delete trailerDict;
 
   outStr->close();
   delete outStr;


More information about the poppler mailing list