[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