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.
> }
>
--
Carlos Garcia Campos
PGP key: http://pgp.mit.edu:11371/pks/lookup?op=get&search=0x523E6462
signature.asc
Description: PGP signature
_______________________________________________ poppler mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/poppler
