El divendres, 12 de maig de 2017, a les 15:31:55 CEST, Carlos Garcia Campos va escriure: > Albert Astals Cid <[email protected]> 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 > > > } _______________________________________________ poppler mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/poppler
