Albert Astals Cid <[email protected]> writes: > El dimarts, 31 de maig de 2016, a les 11:46:38 CEST, Carlos Garcia Campos va > escriure: >> Albert Astals Cid <[email protected]> writes: >> > El dilluns, 30 de maig de 2016, a les 15:43:19 CEST, Carlos Garcia Campos >> > va> >> > escriure: >> >> Albert Astals Cid <[email protected]> writes: >> >> > Hi guys, related to the previous email about std::unique_ptr I think we >> >> > would greatly benefit of a class that makes Object management easier. >> >> > >> >> > Again if you go and check https://bugs.freedesktop.org/attachment.cgi? >> >> > id=124163 we're missing lots of free() and it's hard to prove we won't >> >> > miss >> >> > more. >> >> > >> >> > My suggestion is adding a class called UniqueObject (better name >> >> > welcome) >> >> > >> >> > which will: >> >> > * Call free on itself when it goes out of scope >> >> > >> >> > So we don't need to add .free() in every other >> >> > if-chek-error-return >> >> > >> >> > * Call free on itself when you write on it >> >> > >> >> > So we can do >> >> > >> >> > dict->lookup("Decode", &obj1); >> >> > >> >> > if (obj1.isNull()) { >> >> > >> >> > dict->lookup("D", &obj1); >> >> > >> >> > } >> >> > >> >> > instead of >> >> > >> >> > dict->lookup("Decode", &obj1); >> >> > >> >> > if (obj1.isNull()) { >> >> > >> >> > obj1.free(); >> >> > dict->lookup("D", &obj1); >> >> > >> >> > } >> >> > >> >> > What do you think? >> >> >> >> Why do we need a new class? Wouldn't it be enough to call free in the >> >> Object destructor and init* methods? >> > >> > Not really enough, we need to fix shallowCopy and the operator= that is >> > used randomly in the code, and also the whole rest of the code. >> > >> > I agree this may be the best, to not have two "Object" systems at the same >> > time, but it's also a very big change that needs removing of all the >> > free() >> > calls (well not really since gree of a free object does nothing but still >> > would look weird). >> > >> > If we wanted to do this i guess we would do: >> > * add destructor with free >> > * call free in the initObj define >> > * Rename shallowCopy to be like "takeObject" that would take the internal >> > >> > data and set the other objcet to none >> >> Or better remove shallowCopy and use std::move() >> >> > * Make operator= private so people are forced to use takeObject >> >> Or make the object non-copyable by defining the copy constructor and >> operator= as =delete. >> >> > * Make free() private so we can easily remove all its uses >> > >> > And maybe that would be all? >> >> It seems so >> >> > Sounds doable-ish? >> >> Definitely. > > I did work a bit on this (your C++11 suggestions) and it's doable, it's a bit > tricky in Stream where all the Stream seem to share the same Dict by virtue > of > assigning it around, but i think i'd make it work eventually.
We could even remove copy() and make = always copy. Dicts are refcounted anyway, so copies should be cheap in any case. But I haven't looked at it in detail yet, just proposed ideas without actually trying anything :-P > There's a problem though, doing that we expose C++11 in the "internal" API, > and I'm not sure the consumers of that internal API will be happy. > > In theory we've said multiple times we don't care, that people should be > using > one of the frontends, but that just doesn't happen, and if we break the API > in > a way say libreoffice or latex can't use us anymore we basically make > upgrading poppler almost impossible for distros. > > So i will now try to do it the way I suggested (that is with some more manual > C++), I suggest we start by limiting C++11 to the .cc files for now. > > Am I making sense? I would ask LO and pdflatex devs first, maybe it's not a problem at all for them. > Cheers, > Albert > >> >> > Cheers, >> > >> > Albert >> > >> >> > Cheers, >> >> > >> >> > Albert >> >> > >> >> > _______________________________________________ >> >> > poppler mailing list >> >> > [email protected] >> >> > https://lists.freedesktop.org/mailman/listinfo/poppler > > -- 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
