[poppler] XPDF 3.02 merged in work branch

[Albert Astals Cid]

A Dissabte 14 Abril 2007, Jeff Muizelaar va escriure:
> 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);

Well, why should Dict::set accept "NeedAppearances" and Dict::add not?

It's either all or none, and xpdf has Dict::add as none, so our Dict::set 
is "none" too. Why can change it if you want, but that'll make future merges 
even harder.

Albert

>
> > 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
> _______________________________________________
> poppler mailing list
> poppler at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/poppler