And here is the patch.

It also removes a memory leak in writeTrailer.

Julien


Carlos Garcia Campos wrote:
El mié, 23-01-2008 a las 23:51 +0100, Albert Astals Cid escribió:
A Dimecres 23 Gener 2008, Carlos Garcia Campos va escriure:
El sáb, 19-01-2008 a las 04:39 -0800, Albert Astals Cid escribió:
commit e20f6a8e9ac3936b4bc03710a71fe390dfc4c094
Author: Julien Rebetez <[EMAIL PROTECTED]>
Date:   Sat Jan 19 12:52:02 2008 +0100

    Add deep copy constructor to Dict.

diff --git a/poppler/Dict.cc b/poppler/Dict.cc
index 0c74566..be82890 100644
--- a/poppler/Dict.cc
+++ b/poppler/Dict.cc
@@ -89,7 +101,7 @@ void Dict::set(char *key, Object *val) {
   e = find (key);
   if (e) {
     e->val.free();
-    e->val = *val;
+    val->copy(&e->val);
I think this change is wrong. Dict::set should behave in the same way
whether the key already exists or not. With this change, if the key
already exists, the object will be copied (including dynamically
allocated memory) which means that the original object should be freed
after ->set. However, if the key doesn't exist Dict:add is called and
the object variable is copied (but not the dynamically allocated memory)
which means that the original object shouldn't be freed since it will be
freed by the Dict. Here is an example:

Object obj1;
obj1.initName ("bar");
dict->set ("foo", &obj1);
obj1.free (); -> should we call obj1.free () here? it depends . . .

This it definitely not consistent.
Yeah, it seems we overlooked that, should be remove it until Julien says why he changed it?

well, it doesn't hurt too much, it's leaking memory, but it doesn't
crash.
Albert

   } else {
     add (copyString(key), val);
   }
_______________________________________________
poppler mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/poppler

------------------------------------------------------------------------

_______________________________________________
poppler mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/poppler

>From d1ad2a2a1980e1a5cc3b471da697ae6da29260ba Mon Sep 17 00:00:00 2001
From: Julien Rebetez <[EMAIL PROTECTED]>
Date: Thu, 24 Jan 2008 16:44:23 +0200
Subject: [PATCH] Fix a bug (memory leak) introduced by the "Dict deep copy constructor" patch.
Dict::set is now the same as it was before PDF write patches.
---
 poppler/Dict.cc |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/poppler/Dict.cc b/poppler/Dict.cc
index be82890..df79cca 100644
--- a/poppler/Dict.cc
+++ b/poppler/Dict.cc
@@ -101,7 +101,7 @@ void Dict::set(char *key, Object *val) {
   e = find (key);
   if (e) {
     e->val.free();
-    val->copy(&e->val);
+    e->val = *val;
   } else {
     add (copyString(key), val);
   }
-- 
1.5.2.5


>From 9351b6ce2c3e621fd39f27501b3b847f390de0c8 Mon Sep 17 00:00:00 2001
From: Julien Rebetez <[EMAIL PROTECTED]>
Date: Thu, 24 Jan 2008 17:11:30 +0200
Subject: [PATCH] Remove call to Object::free after adding the Object to Dict because it would cause bugs after
reverting the behavior of Dict::set
---
 poppler/PDFDoc.cc |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/poppler/PDFDoc.cc b/poppler/PDFDoc.cc
index d8d2407..612f320 100644
--- a/poppler/PDFDoc.cc
+++ b/poppler/PDFDoc.cc
@@ -813,12 +813,10 @@ void PDFDoc::writeTrailer (Guint uxrefOffset, int uxrefSize, OutStream* outStr,
 
   obj1.initRef(xref->getRootNum(), xref->getRootGen());
   trailerDict->set("Root", &obj1);
-  obj1.free();
 
   if (incrUpdate) { 
     obj1.initInt(xref->getLastXRefPos());
     trailerDict->set("Prev", &obj1);
-    obj1.free();
   }
   outStr->printf( "trailer\r\n");
   writeDictionnary(trailerDict, outStr);
-- 
1.5.2.5


>From 37927904f4607aa9ec7f93412186570123cf9f2e Mon Sep 17 00:00:00 2001
From: Julien Rebetez <[EMAIL PROTECTED]>
Date: Thu, 24 Jan 2008 17:12:00 +0200
Subject: [PATCH] Fix a memory leak in PDFDoc::writeTrailer

---
 poppler/PDFDoc.cc |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/poppler/PDFDoc.cc b/poppler/PDFDoc.cc
index 612f320..936e03b 100644
--- a/poppler/PDFDoc.cc
+++ b/poppler/PDFDoc.cc
@@ -823,4 +823,6 @@ void PDFDoc::writeTrailer (Guint uxrefOffset, int uxrefSize, OutStream* outStr,
   outStr->printf( "\r\nstartxref\r\n");
   outStr->printf( "%i\r\n", uxrefOffset);
   outStr->printf( "%%%%EOF\r\n");
+
+  delete trailerDict;
 }
-- 
1.5.2.5

_______________________________________________
poppler mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/poppler

Reply via email to