[cairo] Memory leaks in cairomm

[Rodrigo Rivas]

> [snip]
> >>HOWEVER, the current system makes it difficult to cast between related
> >>types (such as Pattern, or the Surface hierarchy of classes).
> >>Cairo::LinearGradient derived = get_some_gradient();
> >>Cairo::Gradient base = derived; //Actually, this would work, though it's
> >>kind of strange.
> >>Cairo::LinearGradient derived2 =
> >>dynamic_cast<Cairo::LinearGradient>(base); //Not valid syntax.
> >
> > Again, I don't see the problem:
> >
> > Cairo::Gradient base = derived; //Ok, ref. counter incremented
>
> This will cut off any data that's in derived but not in the base, though
> we don't have anything like that now. If I derive my own type and do that
> then it will be a problem.

True. This will happen with any C++ hierarchy. The correct way to
downcast is always to use a reference (or pointer):

Cairo::Gradient &base = derived;

>
> > Cairo::LinearGradient linear = base ; //Ok, ref. counter incremented
> >
> > You don't need casts, operator=() makes the needed steps, that is,
> > increment the counter. Sure, you can't be sure if base is actually a
> > LinearGradient, but that's because cairo doesn't tell you that.
>
> a) I think cairo can now. There's new API.
> b) A dynamic_cast<> would use the C++ type information.

a) If there is the API you can assert it, or even set the pointer to
NULL if it doesn't match.
b) Hmmm, dynamic_cast only works for "virtual types", that is, classes
or structs that have at least one virtual function (with a vtable). As
is it, you should use static_cast, and it won't use dynamic type info.

> > [snip]
>
> I don't understand. Const is useful and C++ coders like it. Note that the
> refcount in smartpointers is generally mutable, so that you can refcount
> const pointers.

Of course const is useful!!! I just said that it's not a problem,
because my solution can use it seamlessly. (Actually cairo doesn't use
const with most of their pointers, does it?).

> > IMHO, if cairo manages memory and the lifetime of its objects (with
> > reference counting) is it far better to leave it this way and not to
> > duplicate the effort and the work to build another layer of
> > indirection/managemet over it.
> >
> > Remember the KISS principle.
>
> Sure, the lack of reference-counting of the C++ wrapper would maybe be a
> plus for the implementation. But I still think it makes the API awkward,
> for the reasons stated.

The good thing about my solution is that it is simple and it is a
zero-overhead solution:
 * No waste of memory: the only member variable is the pointer. And no
vtable pointer.
 * no dynamic allocation: all C++ objects can be automatic
 * no extra calls: just increment and decrement the counter
 * no complex and unexpected cloning: if you want to clone an object
you should provide a clone() member function, or similar.

Sure there is drawbacks:
 * there isn't a one-to-one correspondence between C++ wrapper and
cairo objects.
That is: one cairo object can be into many C++ objects. (Actually this
is ok to me, but you can disagree).
 * because of this, you can't implement callbacks with virtual
functions, and clients shouldn't derive classes based on these
wrappers.

Do you still think this API is awkward?
Well, since this is a zero-overhead API, and a fairly useful one (at
least for me, just for lifetime management of cairo objects), I
suggest the following: to create one layer with classes like the one I
described earlier, but adding the Ptr suffix: SurfacePtr, CairoPtr,
GradientPtr, etc. (one for each type of cairo object) that behaves
just like smart pointers / thin wrappers to the cairo objects. Then in
a second layer, you can implement a second layer of objects: Cairo,
Surface, Gradient, that contains the *Ptr and adds more overhead,
member variables, virtual functions, client inheritance, and maybe has
their own lifetime management.

Now if the client wants thin wrapping, of full power they will have it.

--
Rodrigo.