[poppler] XPDF 3.02 merged in work branch

Albert Astals Cid aacid at kde.org
Sat Apr 14 12:31:43 PDT 2007


A Dissabte 14 Abril 2007, Jeff Muizelaar va escriure:
> On Sat, Apr 14, 2007 at 08:58:15PM +0200, Albert Astals Cid wrote:
> > A Dissabte 14 Abril 2007, Jeff Muizelaar va escriure:
> > > 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?
>
> Because Dict::set() calls lookup() first and lookup() does not want a
> copy of the string. This way we only duplicate the string if we need
> to.
>
> If we are going to replace calls to add() with set(), then I agree, they
> should have the same ownership rules for merging's sake. However, if
> this isn't going to happen, it isn't as important to keep the interface
> the same. Either way, I don't have a huge preference. I think Julien is
> the best person to decide whether he wants set() to take ownership or to
> make a copy if necessary.

Agreed.

>
> > 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.
>
> I definitely want add() to take ownership of the data it is passed just
> like xpdf does. Changing it for the UGooString stuff was probably a
> mistake. 

UGooString itself was a [huge] mistake, and i take the blame for it. That's 
why i killed it on 302branch and i take honours for it ;-)

Albert

> Data ownership in poppler/xpdf is not very clear so we 
> shouldn't change the rules unless we have a really good reason to.
>
> -Jeff




More information about the poppler mailing list