[poppler] XPDF 3.02 merged in work branch
Julien Rebetez
julien at fhtagn.net
Fri Apr 20 13:15:20 PDT 2007
On Sat, 2007-04-14 at 15:28 -0400, Jeff Muizelaar wrote:
> 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.
Since Dict::set avoids some code duplication, I think it would be good
to keep it. Basically, there are two solutions :
- Stay with the current version, putting a copyString, as suggested by
Jeff, in the call to add. Perhaps rename or comment it to explain data
ownership more clearly ?
- Make it behave like Dict:add (it takes a newly allocated string) and
delete the string if the find is succesfull.
I think I prefer the first solution because it avoids creating string
that will be deleted immediatly.
Regards,
Julien
More information about the poppler
mailing list