[Poppler-bugs] [Bug 59933] Cleanup patch for bug 50992
bugzilla-daemon at freedesktop.org
bugzilla-daemon at freedesktop.org
Sun Jan 27 06:20:43 PST 2013
https://bugs.freedesktop.org/show_bug.cgi?id=59933
Thomas Freitag <Thomas.Freitag at alfa.de> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #73732|0 |1
is patch| |
--- Comment #1 from Thomas Freitag <Thomas.Freitag at alfa.de> ---
Comment on attachment 73732
--> https://bugs.freedesktop.org/attachment.cgi?id=73732
cleanup code
>diff --git a/goo/GooMutex.h b/goo/GooMutex.h
>index 3f53a62..fbef476 100644
>--- a/goo/GooMutex.h
>+++ b/goo/GooMutex.h
>@@ -60,4 +60,20 @@ typedef pthread_mutex_t GooMutex;
>
> #endif
>
>+namespace Poppler {
>+ enum LockMode {
>+ DoNotLock, // for conditional locks: do not lock
>+ DoLock // for conditional locks: do lock
>+ };
>+
>+ class Lock {
>+ public:
>+ Lock(GooMutex *mutexA) : mutex(mutexA) { mode = DoLock; gLockMutex(mutex); }
>+ Lock(GooMutex *mutexA, LockMode modeA) : mutex(mutexA) { mode = modeA; if (mode == DoLock) gLockMutex(mutex); }
>+ ~Lock() { if (mode == DoLock) gUnlockMutex(mutex); }
>+ private:
>+ GooMutex *mutex;
>+ LockMode mode;
>+ };
>+}
> #endif
>diff --git a/poppler/Annot.cc b/poppler/Annot.cc
>index 0b3c5e4..5bc645a 100644
>--- a/poppler/Annot.cc
>+++ b/poppler/Annot.cc
>@@ -1205,7 +1205,7 @@ void Annot::initialize(PDFDoc *docA, Dict *dict) {
> if (dict->lookupNF("P", &obj1)->isRef()) {
> Ref ref = obj1.getRef();
>
>- page = doc->getCatalog()->findPage (ref.num, ref.gen, gFalse);
>+ page = doc->getCatalog()->findPage (ref.num, ref.gen, Poppler::DoNotLock);
> } else {
> page = 0;
> }
>@@ -3770,7 +3770,7 @@ AnnotWidget::~AnnotWidget() {
> void AnnotWidget::initialize(PDFDoc *docA, Dict *dict) {
> Object obj1;
>
>- form = doc->getCatalog()->getForm(gFalse);
>+ form = doc->getCatalog()->getForm(Poppler::DoNotLock);
>
> if(dict->lookup("H", &obj1)->isName()) {
> const char *modeName = obj1.getName();
>diff --git a/poppler/Array.cc b/poppler/Array.cc
>index d23ed5d..79551e6 100644
>--- a/poppler/Array.cc
>+++ b/poppler/Array.cc
>@@ -35,11 +35,9 @@
> #include "Array.h"
>
> #if MULTITHREADED
>-# define lockArray gLockMutex(&mutex)
>-# define unlockArray gUnlockMutex(&mutex)
>+# define lockArray Poppler::Lock lock(&mutex)
> #else
> # define lockArray
>-# define unlockArray
> #endif
> //------------------------------------------------------------------------
> // Array
>@@ -73,21 +71,18 @@ Object *Array::copy(XRef *xrefA, Object *obj) {
> Object obj1;
> obj->arrayAdd(elems[i].copy(&obj1));
> }
>- unlockArray;
> return obj;
> }
>
> int Array::incRef() {
> lockArray;
> ++ref;
>- unlockArray;
> return ref;
> }
>
> int Array::decRef() {
> lockArray;
> --ref;
>- unlockArray;
> return ref;
> }
>
>@@ -103,7 +98,6 @@ void Array::add(Object *elem) {
> }
> elems[length] = *elem;
> ++length;
>- unlockArray;
> }
>
> void Array::remove(int i) {
>@@ -112,13 +106,11 @@ void Array::remove(int i) {
> #ifdef DEBUG_MEM
> abort();
> #else
>- unlockArray;
> return;
> #endif
> }
> --length;
> memmove( elems + i, elems + i + 1, sizeof(elems[0]) * (length - i) );
>- unlockArray;
> }
>
> Object *Array::get(int i, Object *obj, int recursion) {
>diff --git a/poppler/Catalog.cc b/poppler/Catalog.cc
>index b7f7ede..d4584bc 100644
>--- a/poppler/Catalog.cc
>+++ b/poppler/Catalog.cc
>@@ -57,11 +57,11 @@
> #include "FileSpec.h"
>
> #if MULTITHREADED
>-# define lockCatalog gLockMutex(&mutex)
>-# define unlockCatalog gUnlockMutex(&mutex)
>+# define lockCatalog() Poppler::Lock lock(&mutex)
>+# define condLockCatalog(X) Poppler::Lock condlock(&mutex, (X))
> #else
>-# define lockCatalog
>-# define unlockCatalog
>+# define lockCatalog()
>+# define condLockCatalog(X)
> #endif
> //------------------------------------------------------------------------
> // Catalog
>@@ -191,7 +191,7 @@ GooString *Catalog::readMetadata() {
> Dict *dict;
> Object obj;
>
>- lockCatalog;
>+ lockCatalog();
> if (metadata.isNone()) {
> Object catDict;
>
>@@ -206,7 +206,6 @@ GooString *Catalog::readMetadata() {
> }
>
> if (!metadata.isStream()) {
>- unlockCatalog;
> return NULL;
> }
> dict = metadata.streamGetDict();
>@@ -218,7 +217,6 @@ GooString *Catalog::readMetadata() {
> s = new GooString();
> metadata.getStream()->fillGooString(s);
> metadata.streamClose();
>- unlockCatalog;
> return s;
> }
>
>@@ -226,31 +224,27 @@ Page *Catalog::getPage(int i)
> {
> if (i < 1) return NULL;
>
>- lockCatalog;
>+ lockCatalog();
> if (i > lastCachedPage) {
> GBool cached = cachePageTree(i);
> if ( cached == gFalse) {
>- unlockCatalog;
> return NULL;
> }
> }
>- unlockCatalog;
> return pages[i-1];
> }
>
>-Ref *Catalog::getPageRef(int i, GBool lock)
>+Ref *Catalog::getPageRef(int i, Poppler::LockMode lock)
> {
> if (i < 1) return NULL;
>
>- if (lock) lockCatalog;
>+ condLockCatalog(lock);
> if (i > lastCachedPage) {
> GBool cached = cachePageTree(i);
> if ( cached == gFalse) {
>- if (lock) unlockCatalog;
> return NULL;
> }
> }
>- if (lock) unlockCatalog;
> return &pageRefs[i-1];
> }
>
>@@ -300,7 +294,7 @@ GBool Catalog::cachePageTree(int page)
> return gFalse;
> }
>
>- pagesSize = getNumPages(gFalse);
>+ pagesSize = getNumPages(Poppler::DoLock);
> pages = (Page **)gmallocn(pagesSize, sizeof(Page *));
> pageRefs = (Ref *)gmallocn(pagesSize, sizeof(Ref));
> for (int i = 0; i < pagesSize; ++i) {
>@@ -427,7 +421,7 @@ GBool Catalog::cachePageTree(int page)
> return gFalse;
> }
>
>-int Catalog::findPage(int num, int gen, GBool lock) {
>+int Catalog::findPage(int num, int gen, Poppler::LockMode lock) {
> int i;
>
> for (i = 0; i < getNumPages(lock); ++i) {
>@@ -452,12 +446,11 @@ LinkDest *Catalog::findDest(GooString *name) {
> obj1.free();
> }
> if (!found) {
>- lockCatalog;
>+ lockCatalog();
> if (getDestNameTree()->lookup(name, &obj1))
> found = gTrue;
> else
> obj1.free();
>- unlockCatalog;
> }
> if (!found)
> return NULL;
>@@ -488,7 +481,7 @@ FileSpec *Catalog::embeddedFile(int i)
> {
> Object efDict;
> Object obj;
>- lockCatalog;
>+ lockCatalog();
> obj = getEmbeddedFileNameTree()->getValue(i);
> FileSpec *embeddedFile = 0;
> if (obj.isRef()) {
>@@ -501,7 +494,6 @@ FileSpec *Catalog::embeddedFile(int i)
> Object null;
> embeddedFile = new FileSpec(&null);
> }
>- unlockCatalog;
> return embeddedFile;
> }
>
>@@ -510,12 +502,11 @@ GooString *Catalog::getJS(int i)
> Object obj;
> // getJSNameTree()->getValue(i) returns a shallow copy of the object so we
> // do not need to free it
>- lockCatalog;
>+ lockCatalog();
> getJSNameTree()->getValue(i).fetch(xref, &obj);
>
> if (!obj.isDict()) {
> obj.free();
>- unlockCatalog;
> return 0;
> }
> Object obj2;
>@@ -527,7 +518,6 @@ GooString *Catalog::getJS(int i)
> if (strcmp(obj2.getName(), "JavaScript")) {
> obj2.free();
> obj.free();
>- unlockCatalog;
> return 0;
> }
> obj2.free();
>@@ -543,13 +533,12 @@ GooString *Catalog::getJS(int i)
> }
> obj2.free();
> obj.free();
>- unlockCatalog;
> return js;
> }
>
> Catalog::PageMode Catalog::getPageMode() {
>
>- lockCatalog;
>+ lockCatalog();
> if (pageMode == pageModeNull) {
>
> Object catDict, obj;
>@@ -560,7 +549,6 @@ Catalog::PageMode Catalog::getPageMode() {
> if (!catDict.isDict()) {
> error(errSyntaxError, -1, "Catalog object is wrong type ({0:s})", catDict.getTypeName());
> catDict.free();
>- unlockCatalog;
> return pageMode;
> }
>
>@@ -581,13 +569,12 @@ Catalog::PageMode Catalog::getPageMode() {
> obj.free();
> catDict.free();
> }
>- unlockCatalog;
> return pageMode;
> }
>
> Catalog::PageLayout Catalog::getPageLayout() {
>
>- lockCatalog;
>+ lockCatalog();
> if (pageLayout == pageLayoutNull) {
>
> Object catDict, obj;
>@@ -598,7 +585,6 @@ Catalog::PageLayout Catalog::getPageLayout() {
> if (!catDict.isDict()) {
> error(errSyntaxError, -1, "Catalog object is wrong type ({0:s})", catDict.getTypeName());
> catDict.free();
>- unlockCatalog;
> return pageLayout;
> }
>
>@@ -620,7 +606,6 @@ Catalog::PageLayout Catalog::getPageLayout() {
> obj.free();
> catDict.free();
> }
>- unlockCatalog;
> return pageLayout;
> }
>
>@@ -787,13 +772,9 @@ GBool Catalog::indexToLabel(int index, GooString *label)
> }
> }
>
>-int Catalog::getNumPages(GBool lock)
>+int Catalog::getNumPages(Poppler::LockMode lock)
> {
>- GBool locked = gFalse;
>- if (lock && numPages == -1) {
>- locked = gTrue;
>- lockCatalog;
>- }
>+ condLockCatalog((numPages == -1 && lock == Poppler::DoLock) ? lock : Poppler::DoNotLock);
> if (numPages == -1)
> {
> Object catDict, pagesDict, obj;
>@@ -802,7 +783,6 @@ int Catalog::getNumPages(GBool lock)
> if (!catDict.isDict()) {
> error(errSyntaxError, -1, "Catalog object is wrong type ({0:s})", catDict.getTypeName());
> catDict.free();
>- if (locked) unlockCatalog;
> return 0;
> }
> catDict.dictLookup("Pages", &pagesDict);
>@@ -814,7 +794,6 @@ int Catalog::getNumPages(GBool lock)
> error(errSyntaxError, -1, "Top-level pages object is wrong type ({0:s})",
> pagesDict.getTypeName());
> pagesDict.free();
>- if (locked) unlockCatalog;
> return 0;
> }
>
>@@ -832,13 +811,12 @@ int Catalog::getNumPages(GBool lock)
> pagesDict.free();
> }
>
>- if (locked) unlockCatalog;
> return numPages;
> }
>
> PageLabelInfo *Catalog::getPageLabelInfo()
> {
>- lockCatalog;
>+ lockCatalog();
> if (!pageLabelInfo) {
> Object catDict;
> Object obj;
>@@ -847,24 +825,22 @@ PageLabelInfo *Catalog::getPageLabelInfo()
> if (!catDict.isDict()) {
> error(errSyntaxError, -1, "Catalog object is wrong type ({0:s})", catDict.getTypeName());
> catDict.free();
>- unlockCatalog;
> return NULL;
> }
>
> if (catDict.dictLookup("PageLabels", &obj)->isDict()) {
>- pageLabelInfo = new PageLabelInfo(&obj, getNumPages(gFalse));
>+ pageLabelInfo = new PageLabelInfo(&obj, getNumPages(Poppler::DoLock));
> }
> obj.free();
> catDict.free();
> }
>
>- unlockCatalog;
> return pageLabelInfo;
> }
>
> Object *Catalog::getStructTreeRoot()
> {
>- lockCatalog;
>+ lockCatalog();
> if (structTreeRoot.isNone())
> {
> Object catDict;
>@@ -879,13 +855,12 @@ Object *Catalog::getStructTreeRoot()
> catDict.free();
> }
>
>- unlockCatalog;
> return &structTreeRoot;
> }
>
> Object *Catalog::getOutline()
> {
>- lockCatalog;
>+ lockCatalog();
> if (outline.isNone())
> {
> Object catDict;
>@@ -900,13 +875,12 @@ Object *Catalog::getOutline()
> catDict.free();
> }
>
>- unlockCatalog;
> return &outline;
> }
>
> Object *Catalog::getDests()
> {
>- lockCatalog;
>+ lockCatalog();
> if (dests.isNone())
> {
> Object catDict;
>@@ -921,7 +895,6 @@ Object *Catalog::getDests()
> catDict.free();
> }
>
>- unlockCatalog;
> return &dests;
> }
>
>@@ -943,9 +916,9 @@ Catalog::FormType Catalog::getFormType()
> return res;
> }
>
>-Form *Catalog::getForm(GBool lock)
>+Form *Catalog::getForm(Poppler::LockMode lock)
> {
>- if (lock) lockCatalog;
>+ condLockCatalog(lock);
> if (!form) {
> if (acroForm.isDict()) {
> form = new Form(doc, &acroForm);
>@@ -954,20 +927,18 @@ Form *Catalog::getForm(GBool lock)
> }
> }
>
>- if (lock) unlockCatalog;
> return form;
> }
>
> ViewerPreferences *Catalog::getViewerPreferences()
> {
>- lockCatalog;
>+ lockCatalog();
> if (!viewerPrefs) {
> if (viewerPreferences.isDict()) {
> viewerPrefs = new ViewerPreferences(viewerPreferences.getDict());
> }
> }
>
>- unlockCatalog;
> return viewerPrefs;
> }
>
>diff --git a/poppler/Catalog.h b/poppler/Catalog.h
>index ac7505f..0ddc30a 100644
>--- a/poppler/Catalog.h
>+++ b/poppler/Catalog.h
>@@ -107,13 +107,13 @@ public:
> GBool isOk() { return ok; }
>
> // Get number of pages.
>- int getNumPages(GBool lock = gTrue);
>+ int getNumPages(Poppler::LockMode lock = Poppler::DoLock);
>
> // Get a page.
> Page *getPage(int i);
>
> // Get the reference for a page object.
>- Ref *getPageRef(int i, GBool lock = gTrue);
>+ Ref *getPageRef(int i, Poppler::LockMode lock = Poppler::DoLock);
>
> // Return base URI, or NULL if none.
> GooString *getBaseURI() { return baseURI; }
>@@ -127,7 +127,7 @@ public:
>
> // Find a page, given its object ID. Returns page number, or 0 if
> // not found.
>- int findPage(int num, int gen, GBool lock = gTrue);
>+ int findPage(int num, int gen, Poppler::LockMode lock = Poppler::DoLock);
>
> // Find a named destination. Returns the link destination, or
> // NULL if <name> is not a destination.
>@@ -165,7 +165,7 @@ public:
> };
>
> FormType getFormType();
>- Form* getForm(GBool lock = gTrue);
>+ Form* getForm(Poppler::LockMode lock = Poppler::DoLock);
>
> ViewerPreferences *getViewerPreferences();
>
>@@ -228,7 +228,6 @@ private:
> PageMode pageMode; // page mode
> PageLayout pageLayout; // page layout
>
>- void createPages(); // create pages for caching
> GBool cachePageTree(int page); // Cache first <page> pages.
> Object *findDestInTree(Object *tree, GooString *name, Object *obj);
>
>
--
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/poppler-bugs/attachments/20130127/0040dde4/attachment-0001.html>
More information about the Poppler-bugs
mailing list