[poppler] XPDF 3.02 merged in work branch

Jeff Muizelaar jeff at infidigm.net
Sat Apr 14 12:28:22 PDT 2007


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.

> 
> 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. 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