Hello again, sorry for the rapid succession of patches.
The attached version fixes a linker issue due to non-exported weak symbols and fixes all users of Dict::add to not call copyString and leak memory since std::string will take care of this including SBO. Best regards, Adam. P.S.: Will try to do some performance comparisons against master but this will take at least a day due to the limitations of my machine. Am 08.05.2018 um 08:23 schrieb Adam Reichold: > Also cleaned up the includes a bit... > > Am 08.05.2018 um 08:01 schrieb Adam Reichold: >> Hello again again, >> >> Am 08.05.2018 um 07:37 schrieb Adam Reichold: >>> Hello again, >>> >>> Am 08.05.2018 um 00:12 schrieb Albert Astals Cid: >>>> El dilluns, 7 de maig de 2018, a les 23:03:00 CEST, Adam Reichold va >>>> escriure: >>>>> Hello, >>>>> >>>>> it is probably already too much purely technical code churn for this >>>>> release cycle, but the use of memmove got me thinking how an >>>>> implementation of Dict with more STL usage would look like. >>>>> >>>>> The result is attached, requesting comments on whether this is >>>>> considered useful or not. >>>> >>>> The remove implementation is wrong since it breaks sorting. >>> >>> Could you explain in more detail? My thinking was that any sublist of a >>> sorted list is also sorted and hence removing an element of a list >>> cannot break its sorting? (And since we do not increase the size by >>> removing an element, we cannot reach SORT_LENGTH_LOWER_LIMIT if we were >>> not there already.) >> >> Sorry, that took a minute... Because of the swap... But actually, there >> already is std::vector::erase which does all the tricks like using >> memmove if it is statically safe, so it should actually be simpler. >> >> Fixed version attached... >> >>> Best regards, >>> Adam >>> >>>> Cheers, >>>> Albert >>>> >>>>> >>>>> Best regards, >>>>> Adam >>>>> >>>>> Am 07.05.2018 um 19:14 schrieb Albert Astals Cid: >>>>>> poppler/Array.cc | 2 +- >>>>>> poppler/Dict.cc | 2 +- >>>>>> 2 files changed, 2 insertions(+), 2 deletions(-) >>>>>> >>>>>> New commits: >>>>>> commit 07b8f838b3d8859a3ad34a3140bb24475bd6ac2c >>>>>> Author: Albert Astals Cid <[email protected]> >>>>>> Date: Mon May 7 19:13:07 2018 +0200 >>>>>> >>>>>> cast to void * to bypass new gcc -Wclass-memaccess warning >>>>>> >>>>>> Yes what we're doing there is a bit nasty but it works :D >>>>>> >>>>>> diff --git a/poppler/Array.cc b/poppler/Array.cc >>>>>> index 3276349f..e8aa34ea 100644 >>>>>> --- a/poppler/Array.cc >>>>>> +++ b/poppler/Array.cc >>>>>> @@ -112,7 +112,7 @@ void Array::remove(int i) { >>>>>> >>>>>> #endif >>>>>> >>>>>> } >>>>>> --length; >>>>>> >>>>>> - memmove( elems + i, elems + i + 1, sizeof(elems[0]) * (length - i) ); >>>>>> + memmove( static_cast<void*>(elems + i), elems + i + 1, >>>>>> sizeof(elems[0]) >>>>>> * (length - i) );> >>>>>> } >>>>>> >>>>>> Object Array::get(int i, int recursion) const { >>>>>> >>>>>> diff --git a/poppler/Dict.cc b/poppler/Dict.cc >>>>>> index a431f7eb..bc86fd77 100644 >>>>>> --- a/poppler/Dict.cc >>>>>> +++ b/poppler/Dict.cc >>>>>> @@ -201,7 +201,7 @@ void Dict::remove(const char *key) { >>>>>> >>>>>> gfree(entries[pos].key); >>>>>> entries[pos].val.free(); >>>>>> if (pos != length) { >>>>>> >>>>>> - memmove(&entries[pos], &entries[pos + 1], (length - pos) * >>>>>> sizeof(DictEntry)); + memmove(static_cast<void*>(&entries[pos]), >>>>>> &entries[pos + 1], (length - pos) * sizeof(DictEntry));> >>>>>> } >>>>>> >>>>>> } >>>>>> >>>>>> } else { >>>>>> >>>>>> _______________________________________________ >>>>>> poppler mailing list >>>>>> [email protected] >>>>>> https://lists.freedesktop.org/mailman/listinfo/poppler >>>> >>>> >>>> >>>> >>>> _______________________________________________ >>>> poppler mailing list >>>> [email protected] >>>> https://lists.freedesktop.org/mailman/listinfo/poppler >>>> >>> >>> >>> >>> _______________________________________________ >>> poppler mailing list >>> [email protected] >>> https://lists.freedesktop.org/mailman/listinfo/poppler >>> >>> >>> >>> _______________________________________________ >>> poppler mailing list >>> [email protected] >>> https://lists.freedesktop.org/mailman/listinfo/poppler >>> >>> >>> _______________________________________________ >>> poppler mailing list >>> [email protected] >>> https://lists.freedesktop.org/mailman/listinfo/poppler
From 62b907bd20dda803b1cddf28eda8e07500a84ea0 Mon Sep 17 00:00:00 2001 From: Adam Reichold <[email protected]> Date: Mon, 7 May 2018 22:58:43 +0200 Subject: [PATCH 1/4] Try to simplify Dict by implementing it in terms of std::vector and std::string. --- poppler/Catalog.cc | 2 +- poppler/Catalog.h | 2 +- poppler/Dict.cc | 240 ++++++++++++++++------------------------------------- poppler/Dict.h | 46 +++++----- poppler/Form.cc | 2 +- poppler/Object.h | 4 +- 6 files changed, 99 insertions(+), 197 deletions(-) diff --git a/poppler/Catalog.cc b/poppler/Catalog.cc index d259bda6..eba626e6 100644 --- a/poppler/Catalog.cc +++ b/poppler/Catalog.cc @@ -428,7 +428,7 @@ int Catalog::numDests() return obj->dictGetLength(); } -char *Catalog::getDestsName(int i) +const char *Catalog::getDestsName(int i) { Object *obj; diff --git a/poppler/Catalog.h b/poppler/Catalog.h index 0467159a..2c18b4a2 100644 --- a/poppler/Catalog.h +++ b/poppler/Catalog.h @@ -163,7 +163,7 @@ public: int numDests(); // Get the i'th named destination name in name-dict - char *getDestsName(int i); + const char *getDestsName(int i); // Get the i'th named destination link destination in name-dict LinkDest *getDestsDest(int i); diff --git a/poppler/Dict.cc b/poppler/Dict.cc index bc86fd77..5cf2cb25 100644 --- a/poppler/Dict.cc +++ b/poppler/Dict.cc @@ -35,10 +35,7 @@ #endif #include <algorithm> -#include <stddef.h> -#include <string.h> -#include "goo/gmem.h" -#include "Object.h" + #include "XRef.h" #include "Dict.h" @@ -51,247 +48,150 @@ // Dict //------------------------------------------------------------------------ -static const int SORT_LENGTH_LOWER_LIMIT = 32; - -static inline bool cmpDictEntries(const DictEntry &e1, const DictEntry &e2) -{ - return strcmp(e1.key, e2.key) < 0; -} +constexpr int SORT_LENGTH_LOWER_LIMIT = 32; -static int binarySearch(const char *key, DictEntry *entries, int length) -{ - int first = 0; - int end = length - 1; - while (first <= end) { - const int middle = (first + end) / 2; - const int res = strcmp(key, entries[middle].key); - if (res == 0) { - return middle; - } else if (res < 0) { - end = middle - 1; - } else { - first = middle + 1; - } +struct Dict::CmpDictEntry { + bool operator()(const DictEntry &lhs, const DictEntry &rhs) const { + return lhs.first < rhs.first; } - return -1; -} + bool operator()(const DictEntry &lhs, const char *rhs) const { + return lhs.first < rhs; + } + bool operator()(const char *lhs, const DictEntry &rhs) const { + return lhs < rhs.first; + } +}; Dict::Dict(XRef *xrefA) { xref = xrefA; - entries = nullptr; - size = length = 0; ref = 1; - sorted = gFalse; #ifdef MULTITHREADED gInitMutex(&mutex); #endif + + sorted = false; } -Dict::Dict(Dict* dictA) { +Dict::Dict(const Dict* dictA) { xref = dictA->xref; - size = length = dictA->length; ref = 1; #ifdef MULTITHREADED gInitMutex(&mutex); #endif - sorted = dictA->sorted; - entries = (DictEntry *)gmallocn(size, sizeof(DictEntry)); - for (int i=0; i<length; i++) { - entries[i].key = copyString(dictA->entries[i].key); - entries[i].val.initNullAfterMalloc(); - entries[i].val = dictA->entries[i].val.copy(); + entries.reserve(dictA->entries.size()); + for (const auto& entry : dictA->entries) { + entries.emplace_back(entry.first, entry.second.copy()); } + + sorted = dictA->sorted; } -Dict *Dict::copy(XRef *xrefA) { +Dict *Dict::copy(XRef *xrefA) const { dictLocker(); Dict *dictA = new Dict(this); dictA->xref = xrefA; - for (int i=0; i<length; i++) { - if (dictA->entries[i].val.getType() == objDict) { - Dict *copy = dictA->entries[i].val.getDict()->copy(xrefA); - dictA->entries[i].val = Object(copy); + for (auto &entry : dictA->entries) { + if (entry.second.getType() == objDict) { + entry.second = Object(entry.second.getDict()->copy(xrefA)); } } return dictA; } Dict::~Dict() { - int i; - - for (i = 0; i < length; ++i) { - gfree(entries[i].key); - entries[i].val.free(); - } - gfree(entries); #ifdef MULTITHREADED gDestroyMutex(&mutex); #endif } -int Dict::incRef() { +void Dict::add(const char *key, Object &&val) { dictLocker(); - ++ref; - return ref; -} - -int Dict::decRef() { - dictLocker(); - --ref; - return ref; -} - -void Dict::add(char *key, Object &&val) { - dictLocker(); - if (sorted) { - // We use add on very few occasions so - // virtually this will never be hit - sorted = gFalse; - } - - if (length == size) { - if (length == 0) { - size = 8; - } else { - size *= 2; + if (entries.size() >= SORT_LENGTH_LOWER_LIMIT) { + if (!sorted) { + std::sort(entries.begin(), entries.end(), CmpDictEntry{}); + sorted = true; } - entries = (DictEntry *)greallocn(entries, size, sizeof(DictEntry)); + const auto pos = std::upper_bound(entries.begin(), entries.end(), key, CmpDictEntry{}); + entries.emplace(pos, key, std::move(val)); + } else { + entries.emplace_back(key, std::move(val)); + sorted = false; } - entries[length].key = key; - entries[length].val.initNullAfterMalloc(); - entries[length].val = std::move(val); - ++length; } -inline DictEntry *Dict::find(const char *key) const { - if (!sorted && length >= SORT_LENGTH_LOWER_LIMIT) - { - dictLocker(); - sorted = gTrue; - std::sort(entries, entries+length, cmpDictEntries); - } - +inline const Dict::DictEntry *Dict::find(const char *key) const { if (sorted) { - const int pos = binarySearch(key, entries, length); - if (pos != -1) { - return &entries[pos]; + const auto pos = std::lower_bound(entries.begin(), entries.end(), key, CmpDictEntry{}); + if (pos != entries.end() && pos->first == key) { + return &*pos; } } else { - int i; - - for (i = length - 1; i >=0; --i) { - if (!strcmp(key, entries[i].key)) - return &entries[i]; + const auto pos = std::find_if(entries.rbegin(), entries.rend(), [key](const DictEntry& entry) { + return entry.first == key; + }); + if (pos != entries.rend()) { + return &*pos; } } return nullptr; } -GBool Dict::hasKey(const char *key) const { - return find(key) != nullptr; +inline Dict::DictEntry *Dict::find(const char *key) { + return const_cast<DictEntry *>(const_cast<const Dict *>(this)->find(key)); } void Dict::remove(const char *key) { dictLocker(); - if (sorted) { - const int pos = binarySearch(key, entries, length); - if (pos != -1) { - length -= 1; - gfree(entries[pos].key); - entries[pos].val.free(); - if (pos != length) { - memmove(static_cast<void*>(&entries[pos]), &entries[pos + 1], (length - pos) * sizeof(DictEntry)); - } - } - } else { - int i; - bool found = false; - if(length == 0) { - return; - } - - for(i=0; i<length; i++) { - if (!strcmp(key, entries[i].key)) { - found = true; - break; - } - } - if(!found) { - return; - } - //replace the deleted entry with the last entry - gfree(entries[i].key); - entries[i].val.free(); - length -= 1; - if (i!=length) { - //don't copy the last entry if it is deleted - entries[i].key = entries[length].key; - entries[i].val = std::move(entries[length].val); - } + if (auto *entry = find(key)) { + entries.erase(std::vector<DictEntry>::iterator{entry}); } } void Dict::set(const char *key, Object &&val) { - DictEntry *e; if (val.isNull()) { remove(key); return; } - e = find (key); - if (e) { - dictLocker(); - e->val = std::move(val); + dictLocker(); + if (auto *entry = find(key)) { + entry->second = std::move(val); } else { - add (copyString(key), std::move(val)); + add(key, std::move(val)); } } GBool Dict::is(const char *type) const { - DictEntry *e; - - return (e = find("Type")) && e->val.isName(type); + if (const auto *entry = find("Type")) { + return entry->second.isName(type); + } + return gFalse; } Object Dict::lookup(const char *key, int recursion) const { - DictEntry *e; - - return (e = find(key)) ? e->val.fetch(xref, recursion) : Object(objNull); + if (const auto *entry = find(key)) { + return entry->second.fetch(xref, recursion); + } + return Object(objNull); } Object Dict::lookupNF(const char *key) const { - DictEntry *e; - - return (e = find(key)) ? e->val.copy() : Object(objNull); + if (const auto *entry = find(key)) { + return entry->second.copy(); + } + return Object(objNull); } GBool Dict::lookupInt(const char *key, const char *alt_key, int *value) const { - GBool success = gFalse; - Object obj1 = lookup ((char *) key); - if (obj1.isNull () && alt_key != nullptr) { - obj1.free (); - obj1 = lookup ((char *) alt_key); + auto obj1 = lookup(key); + if (obj1.isNull() && alt_key != nullptr) { + obj1 = lookup(alt_key); } - if (obj1.isInt ()) { - *value = obj1.getInt (); - success = gTrue; + if (obj1.isInt()) { + *value = obj1.getInt(); + return gTrue; } - - obj1.free (); - - return success; -} - -char *Dict::getKey(int i) const { - return entries[i].key; -} - -Object Dict::getVal(int i) const { - return entries[i].val.fetch(xref); -} - -Object Dict::getValNF(int i) const { - return entries[i].val.copy(); + return gFalse; } diff --git a/poppler/Dict.h b/poppler/Dict.h index 9fd732a1..276d8c58 100644 --- a/poppler/Dict.h +++ b/poppler/Dict.h @@ -33,6 +33,11 @@ #pragma interface #endif +#include <atomic> +#include <string> +#include <vector> +#include <utility> + #include "poppler-config.h" #include "Object.h" #include "goo/GooMutex.h" @@ -41,18 +46,13 @@ // Dict //------------------------------------------------------------------------ -struct DictEntry { - char *key; - Object val; -}; - class Dict { public: // Constructor. Dict(XRef *xrefA); - Dict(Dict* dictA); - Dict *copy(XRef *xrefA); + Dict(const Dict *dictA); + Dict *copy(XRef *xrefA) const; // Destructor. ~Dict(); @@ -61,11 +61,11 @@ public: Dict& operator=(const Dict &) = delete; // Get number of entries. - int getLength() const { return length; } + int getLength() const { return static_cast<int>(entries.size()); } - // Add an entry. NB: does not copy key. + // Add an entry. // val becomes a dead object after the call - void add(char *key, Object &&val); + void add(const char *key, Object &&val); // Update the value of an existing entry, otherwise create it // val becomes a dead object after the call @@ -83,9 +83,9 @@ public: GBool lookupInt(const char *key, const char *alt_key, int *value) const; // Iterative accessors. - char *getKey(int i) const; - Object getVal(int i) const; - Object getValNF(int i) const; + const char *getKey(int i) const { return entries[i].first.c_str(); } + Object getVal(int i) const { return entries[i].second.fetch(xref); } + Object getValNF(int i) const { return entries[i].second.copy(); } // Set the xref pointer. This is only used in one special case: the // trailer dictionary, which is read before the xref table is @@ -94,26 +94,28 @@ public: XRef *getXRef() const { return xref; } - GBool hasKey(const char *key) const; + GBool hasKey(const char *key) const { return find(key) != nullptr; } private: friend class Object; // for incRef/decRef // Reference counting. - int incRef(); - int decRef(); + int incRef() { return ++ref; } + int decRef() { return --ref; } + + using DictEntry = std::pair<std::string, Object>; + struct CmpDictEntry; - mutable GBool sorted; + bool sorted; XRef *xref; // the xref table for this PDF file - DictEntry *entries; // array of entries - int size; // size of <entries> array - int length; // number of entries in dictionary - int ref; // reference count + std::vector<DictEntry> entries; + std::atomic_int ref; // reference count #ifdef MULTITHREADED mutable GooMutex mutex; #endif - DictEntry *find(const char *key) const; + const DictEntry *find(const char *key) const; + DictEntry *find(const char *key); }; #endif diff --git a/poppler/Form.cc b/poppler/Form.cc index 820944e8..a4aab1a5 100644 --- a/poppler/Form.cc +++ b/poppler/Form.cc @@ -193,7 +193,7 @@ FormWidgetButton::FormWidgetButton (PDFDoc *docA, Object *aobj, unsigned num, Re Object obj2 = obj1.dictLookup("N"); if (obj2.isDict()) { for (int i = 0; i < obj2.dictGetLength(); i++) { - char *key = obj2.dictGetKey(i); + const char *key = obj2.dictGetKey(i); if (strcmp (key, "Off") != 0) { onStr = new GooString (key); break; diff --git a/poppler/Object.h b/poppler/Object.h index 97978f46..bcfd8ba9 100644 --- a/poppler/Object.h +++ b/poppler/Object.h @@ -269,7 +269,7 @@ public: GBool dictIs(const char *dictType) const; Object dictLookup(const char *key, int recursion = 0) const; Object dictLookupNF(const char *key) const; - char *dictGetKey(int i) const; + const char *dictGetKey(int i) const; Object dictGetVal(int i) const; Object dictGetValNF(int i) const; @@ -374,7 +374,7 @@ inline Object Object::dictLookup(const char *key, int recursion) const inline Object Object::dictLookupNF(const char *key) const { OBJECT_TYPE_CHECK(objDict); return dict->lookupNF(key); } -inline char *Object::dictGetKey(int i) const +inline const char *Object::dictGetKey(int i) const { OBJECT_TYPE_CHECK(objDict); return dict->getKey(i); } inline Object Object::dictGetVal(int i) const -- 2.16.3 From 7612e2462de26030739b2265d99b413347f89a82 Mon Sep 17 00:00:00 2001 From: Adam Reichold <[email protected]> Date: Tue, 8 May 2018 08:32:03 +0200 Subject: [PATCH 2/4] But do use swap-and-pop if the Dict is still small enough to be unsorted. --- poppler/Dict.cc | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/poppler/Dict.cc b/poppler/Dict.cc index 5cf2cb25..b50a22fc 100644 --- a/poppler/Dict.cc +++ b/poppler/Dict.cc @@ -144,7 +144,12 @@ inline Dict::DictEntry *Dict::find(const char *key) { void Dict::remove(const char *key) { dictLocker(); if (auto *entry = find(key)) { - entries.erase(std::vector<DictEntry>::iterator{entry}); + if (sorted) { + entries.erase(std::vector<DictEntry>::iterator{entry}); + } else { + swap(*entry, entries.back()); + entries.pop_back(); + } } } -- 2.16.3 From 6fe8f21fb87fa774417586a3b1dd1a39587556ce Mon Sep 17 00:00:00 2001 From: Adam Reichold <[email protected]> Date: Tue, 8 May 2018 18:19:03 +0200 Subject: [PATCH 3/4] Fix linking errors in release builds due to non exported weak symbols. --- poppler/Dict.cc | 4 ++++ poppler/Dict.h | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/poppler/Dict.cc b/poppler/Dict.cc index b50a22fc..47a66528 100644 --- a/poppler/Dict.cc +++ b/poppler/Dict.cc @@ -200,3 +200,7 @@ GBool Dict::lookupInt(const char *key, const char *alt_key, int *value) const } return gFalse; } + +GBool Dict::hasKey(const char *key) const { + return find(key) != nullptr; +} diff --git a/poppler/Dict.h b/poppler/Dict.h index 276d8c58..da7547a9 100644 --- a/poppler/Dict.h +++ b/poppler/Dict.h @@ -94,7 +94,7 @@ public: XRef *getXRef() const { return xref; } - GBool hasKey(const char *key) const { return find(key) != nullptr; } + GBool hasKey(const char *key) const; private: friend class Object; // for incRef/decRef -- 2.16.3 From 8a64d4cf7f78d5d0adb8d9d92122fb9b6c6f3903 Mon Sep 17 00:00:00 2001 From: Adam Reichold <[email protected]> Date: Tue, 8 May 2018 18:54:32 +0200 Subject: [PATCH 4/4] Adjust all users of Dict::add to avoid leaking memory due to by now unnecessary calls to copyString. --- poppler/Annot.cc | 28 ++++++++++++++-------------- poppler/Gfx.cc | 9 +++------ poppler/Object.h | 4 ++-- poppler/PDFDoc.cc | 8 ++++---- poppler/Parser.cc | 8 +++----- utils/pdfunite.cc | 4 ++-- 6 files changed, 28 insertions(+), 33 deletions(-) diff --git a/poppler/Annot.cc b/poppler/Annot.cc index d5a66524..b7f4655c 100644 --- a/poppler/Annot.cc +++ b/poppler/Annot.cc @@ -2808,14 +2808,14 @@ static GfxFont * createAnnotDrawFont(XRef * xref, Dict *fontResDict) const Ref dummyRef = { -1, -1 }; Dict *fontDict = new Dict(xref); - fontDict->add(copyString("BaseFont"), Object(objName, "Helvetica")); - fontDict->add(copyString("Subtype"), Object(objName, "Type0")); - fontDict->add(copyString("Encoding"), Object(objName, "WinAnsiEncoding")); + fontDict->add("BaseFont", Object(objName, "Helvetica")); + fontDict->add("Subtype", Object(objName, "Type0")); + fontDict->add("Encoding", Object(objName, "WinAnsiEncoding")); Dict *fontsDict = new Dict(xref); - fontsDict->add(copyString("AnnotDrawFont"), Object(fontDict)); + fontsDict->add("AnnotDrawFont", Object(fontDict)); - fontResDict->add(copyString("Font"), Object(fontsDict)); + fontResDict->add("Font", Object(fontsDict)); return GfxFont::makeFont(xref, "AnnotDrawFont", dummyRef, fontDict); } @@ -4881,19 +4881,19 @@ void AnnotWidget::generateFieldAppearance(bool *addedDingbatsResource) { const GooString *appearBuf = appearBuilder.buffer(); // build the appearance stream dictionary Dict *appearDict = new Dict(xref); - appearDict->add(copyString("Length"), Object(appearBuf->getLength())); - appearDict->add(copyString("Subtype"), Object(objName, "Form")); + appearDict->add("Length", Object(appearBuf->getLength())); + appearDict->add("Subtype", Object(objName, "Form")); Array *bbox = new Array(xref); bbox->add(Object(0)); bbox->add(Object(0)); bbox->add(Object(rect->x2 - rect->x1)); bbox->add(Object(rect->y2 - rect->y1)); - appearDict->add(copyString("BBox"), Object(bbox)); + appearDict->add("BBox", Object(bbox)); // set the resource dictionary Object *resDict = form->getDefaultResourcesObj(); if (resDict->isDict()) { - appearDict->add(copyString("Resources"), resDict->copy()); + appearDict->add("Resources", resDict->copy()); } // build the appearance stream @@ -4932,7 +4932,7 @@ void AnnotWidget::updateAppearanceStream() // Write the AP dictionary obj1 = Object(new Dict(xref)); - obj1.dictAdd(copyString("N"), Object(updatedAppearanceStream.num, updatedAppearanceStream.gen)); + obj1.dictAdd("N", Object(updatedAppearanceStream.num, updatedAppearanceStream.gen)); // Update our internal pointers to the appearance dictionary appearStreams = new AnnotAppearance(doc, &obj1); @@ -4965,14 +4965,14 @@ void AnnotWidget::draw(Gfx *gfx, GBool printing) { // We are forcing ZaDb but the font does not exist // so create a fake one Dict *fontDict = new Dict(gfx->getXRef()); - fontDict->add(copyString("BaseFont"), Object(objName, "ZapfDingbats")); - fontDict->add(copyString("Subtype"), Object(objName, "Type1")); + fontDict->add("BaseFont", Object(objName, "ZapfDingbats")); + fontDict->add("Subtype", Object(objName, "Type1")); Dict *fontsDict = new Dict(gfx->getXRef()); - fontsDict->add(copyString("ZaDb"), Object(fontDict)); + fontsDict->add("ZaDb", Object(fontDict)); Dict *dict = new Dict(gfx->getXRef()); - dict->add(copyString("Font"), Object(fontsDict)); + dict->add("Font", Object(fontsDict)); gfx->pushResources(dict); delete dict; } diff --git a/poppler/Gfx.cc b/poppler/Gfx.cc index a4d12a70..73a48758 100644 --- a/poppler/Gfx.cc +++ b/poppler/Gfx.cc @@ -4903,7 +4903,6 @@ void Gfx::opBeginImage(Object args[], int numArgs) { } Stream *Gfx::buildImageStream() { - char *key; Stream *str; // build dictionary @@ -4913,13 +4912,11 @@ Stream *Gfx::buildImageStream() { if (!obj.isName()) { error(errSyntaxError, getPos(), "Inline image dictionary key must be a name object"); } else { - key = copyString(obj.getName()); - obj = parser->getObj(); - if (obj.isEOF() || obj.isError()) { - gfree(key); + auto val = parser->getObj(); + if (val.isEOF() || val.isError()) { break; } - dict.dictAdd(key, std::move(obj)); + dict.dictAdd(obj.getName(), std::move(val)); } obj = parser->getObj(); } diff --git a/poppler/Object.h b/poppler/Object.h index bcfd8ba9..d2ebdf55 100644 --- a/poppler/Object.h +++ b/poppler/Object.h @@ -263,7 +263,7 @@ public: // Dict accessors. int dictGetLength() const; - void dictAdd(char *key, Object &&val); + void dictAdd(const char *key, Object &&val); void dictSet(const char *key, Object &&val); void dictRemove(const char *key); GBool dictIs(const char *dictType) const; @@ -353,7 +353,7 @@ inline Object Object::arrayGetNF(int i) const inline int Object::dictGetLength() const { OBJECT_TYPE_CHECK(objDict); return dict->getLength(); } -inline void Object::dictAdd(char *key, Object &&val) +inline void Object::dictAdd(const char *key, Object &&val) { OBJECT_TYPE_CHECK(objDict); dict->add(key, std::move(val)); } inline void Object::dictSet(const char *key, Object &&val) diff --git a/poppler/PDFDoc.cc b/poppler/PDFDoc.cc index 631f9a70..01f74e79 100644 --- a/poppler/PDFDoc.cc +++ b/poppler/PDFDoc.cc @@ -1645,7 +1645,7 @@ void PDFDoc::replacePageDict(int pageNo, int rotate, mediaBoxArray->add(Object(mediaBox->y2)); Object mediaBoxObject(mediaBoxArray); Object trimBoxObject = mediaBoxObject.copy(); - pageDict->add(copyString("MediaBox"), std::move(mediaBoxObject)); + pageDict->add("MediaBox", std::move(mediaBoxObject)); if (cropBox != nullptr) { Array *cropBoxArray = new Array(getXRef()); cropBoxArray->add(Object(cropBox->x1)); @@ -1654,10 +1654,10 @@ void PDFDoc::replacePageDict(int pageNo, int rotate, cropBoxArray->add(Object(cropBox->y2)); Object cropBoxObject(cropBoxArray); trimBoxObject = cropBoxObject.copy(); - pageDict->add(copyString("CropBox"), std::move(cropBoxObject)); + pageDict->add("CropBox", std::move(cropBoxObject)); } - pageDict->add(copyString("TrimBox"), std::move(trimBoxObject)); - pageDict->add(copyString("Rotate"), Object(rotate)); + pageDict->add("TrimBox", std::move(trimBoxObject)); + pageDict->add("Rotate", Object(rotate)); getXRef()->setModifiedObject(&page, *refPage); } diff --git a/poppler/Parser.cc b/poppler/Parser.cc index 869e94ad..a9a975ad 100644 --- a/poppler/Parser.cc +++ b/poppler/Parser.cc @@ -112,20 +112,18 @@ Object Parser::getObj(GBool simpleOnly, if (strict) goto err; shift(); } else { - // buf1 might go away in shift(), so construct the key - char *key = copyString(buf1.getName()); + // buf1 will go away in shift(), so keep the key + const auto key = std::move(buf1); shift(); if (buf1.isEOF() || buf1.isError()) { - gfree(key); if (strict && buf1.isError()) goto err; break; } Object obj2 = getObj(gFalse, fileKey, encAlgorithm, keyLength, objNum, objGen, recursion + 1); if (unlikely(obj2.isError() && recursion + 1 >= recursionLimit)) { - gfree(key); break; } - obj.dictAdd(key, std::move(obj2)); + obj.dictAdd(key.getName(), std::move(obj2)); } } if (buf1.isEOF()) { diff --git a/utils/pdfunite.cc b/utils/pdfunite.cc index 0bdfbdf6..a5dc4d05 100644 --- a/utils/pdfunite.cc +++ b/utils/pdfunite.cc @@ -91,7 +91,7 @@ static void doMergeNameTree(PDFDoc *doc, XRef *srcXRef, XRef *countRef, int oldR newNameArray->add(Object(value.getRef().num + numOffset, value.getRef().gen)); } } - srcNameTree->add(copyString("Names"), Object(newNameArray)); + srcNameTree->add("Names", Object(newNameArray)); doc->markPageObjects(mergeNameTree, srcXRef, countRef, numOffset, oldRefNum, newRefNum); } } @@ -106,7 +106,7 @@ static void doMergeNameDict(PDFDoc *doc, XRef *srcXRef, XRef *countRef, int oldR } else if (srcNameTree.isNull() && mergeNameTree.isDict()) { Object newNameTree(new Dict(srcXRef)); doMergeNameTree(doc, srcXRef, countRef, oldRefNum, newRefNum, newNameTree.getDict(), mergeNameTree.getDict(), numOffset); - srcNameDict->add(copyString(key), std::move(newNameTree)); + srcNameDict->add(key, std::move(newNameTree)); } } } -- 2.16.3
signature.asc
Description: OpenPGP digital signature
_______________________________________________ poppler mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/poppler
