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 * Make operator= private so people are forced to use takeObject * Make free() private so we can easily remove all its uses And maybe that would be all? Sounds doable-ish? Cheers, Albert > > > Cheers, > > > > Albert > > > > _______________________________________________ > > poppler mailing list > > [email protected] > > https://lists.freedesktop.org/mailman/listinfo/poppler _______________________________________________ poppler mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/poppler
