Fri May 12 21:45:25 UTC 2017

El divendres, 12 de maig de 2017, a les 15:31:55 CEST, Carlos Garcia Campos va 
escriure:
> Albert Astals Cid <aacid at kemper.freedesktop.org> writes:
> 
> 
> This looks great, I have a couple of comments after a first glance.
> 
> >  	  if (!found)
> > 
> > -	    annotObj.free ();
> > +	    annotObj.setToNull ();
> 
> As I commented on IRC, I think
> 
> annotObj = Object(objNull);
> 
> is not less efficient, looks simpler and we don't need setToNull() at
> all.
> 
> > diff --git a/poppler/Annot.cc b/poppler/Annot.cc
> > index 73c2fe66..96a7e330 100644
> > --- a/poppler/Annot.cc
> > +++ b/poppler/Annot.cc
> > 
> > @@ -1228,12 +1188,13 @@ Annot::Annot(PDFDoc *docA, Dict *dict, Object
> > *obj) {> 
> >    }
> >    flags = flagUnknown;
> >    type = typeUnknown;
> > 
> > -  annotObj.initDict (dict);
> > +  dict->incRef();
> > +  annotObj = Object(dict);
> > 
> >    initialize (docA, dict);
> 
> I still find this pattern quite confusing. The problem is that Object()
> is taking the ownership of passing refcounted objects. That's not
> consistent with all other types where the value is copied. I think it
> should be Object() constructor the one doing the incRef() in case of
> dicts, arrays and streams.

You just complained exactly about the reverse thing (needing to do a decRef 
when passing a Dict to an Object) in the other email ;)

I've done a few changes making all the incRef/decRef private.

Now the the pattern is:
 * If you pass a Dict/Annot/Array to Object it will "take" it, that's most of 
the times correct because if you do that is because you just new'ed the Dict/
Annot/Array yourself
 * If what you want to do is incRef, you should be passing Objects and not raw 
Dict/Annot/Array

Does that work for you?

Cheers,
  Albert

> 
> >  }