[poppler] Object management helper class

[Albert Astals Cid]

El dilluns, 30 de maig de 2016, a les 15:43:19 CEST, Carlos Garcia Campos va 
escriure:
> Albert Astals Cid <aacid at kde.org> 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
> > poppler at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/poppler