[poppler] XPDF 3.02 merged in work branch

Jeff Muizelaar jeff at infidigm.net
Sat Apr 14 10:55:03 PDT 2007


On Sat, Apr 14, 2007 at 01:10:16PM +0200, Julien Rebetez wrote:
> On Sat, 2007-04-14 at 00:58 -0400, Jeff Muizelaar wrote:
> > On Fri, Apr 13, 2007 at 08:55:23PM -0400, Jeff Muizelaar wrote:
> > > On Fri, Apr 13, 2007 at 08:45:41PM -0400, Jeff Muizelaar wrote:
> > > > On Fri, Apr 13, 2007 at 05:11:48PM +0200, Julien Rebetez wrote:
> > > > > On Fri, 2007-04-06 at 16:30 +0200, Albert Astals Cid wrote: 
> > > > > > * Merge Annot [1], Julien?
> > > > > 
> > > > > The attached patch should do the job.
> > > > > Aside from merging xpdf 3.02 Annot-related changes, it fixes a bug with
> > > > > an assert in GooString's constructor.
> > > > 
> > > > http://infidigm.net/~jeff/forms/Javascripts.pdf still crashes with this
> > > > patch applied.
> > > >
> > > It looks like the problem is with these lines:
> > > Annot.cc:      appearDict.dictAdd("Resources", drObj.copy(&resObj));
> > > Annot.cc:    appearDict.dictAdd("Length", obj1.initInt(appearBuf->getLength()));
> > > Annot.cc:    appearDict.dictAdd("Subtype", obj1.initName("Form"));
> > > Annot.cc:    appearDict.dictAdd("BBox", &obj1);
> > > 
> > > They all add an entry to the dictionary using a constant key.
> > > However, in Dict.cc:34 we have gfree(entries[i].key) which is obviously
> > > problematic.
> > > 
> > > An easy solution would be to use something like gstrdup(). I'm not sure
> > > what the right solution is.
> > 
> > Looks like the right thing to do is copyString("blah").
> > 
> These problematic lines are removed by the patch I just submitted and
> all the dictAdd with constant string are done through copyString. ( I
> should have said my patch was done against the xpdf302merge branch. )

Yep, turns out I was looking at the wrong source tree. However, I'm
still getting crashes. It looks like the problem is actually:

poppler/Form.cc:  acroForm->dictSet("NeedAppearances", &obj1);
poppler/Form.cc:  catalog->dictSet("AcroForm", acroForm);

Currently Dict::set() is an unusable interface. It should probably do:
    add (copyString(key), val);
instead of:
    add (key, val);

> All your forms test files are displayed correctly with this patch on my
> machine.

I wonder why you weren't seeing the crashes, maybe you were just getting
lucky...

> 
> Perhaps these lines are the cause of the crash in HEAD though, I'll look
> into it.

HEAD doesn't crash because it's dictAdd makes a copy of the string.
HEAD's problem is rendering regressions.

-Jeff


More information about the poppler mailing list