2009/6/1 Gustavo Sverzut Barbieri <barbieri@...>:
> 2009/5/29 André Loureiro <loureiro.andrew@...>:
>> Hi guys,
>> I've made the bindings for epdf library, and I would like put it
>> in your svn repository, how can i do this?
>> the current repository is in gitorious.org, but I think that is not
>> better place.
> Hi André,
> Before I merge your work inside SVN I need you to fix some problems with it:
> - release the GIL in functions that (possibly) take long, like
> render(). See http://svn.enlightenment.org/svn/e/trunk/BINDINGS/python/python-ecore/ecore/ecore.c_ecore.pyx
> (Reminds me that I need to add such to evas render as well, if you
> could submit a patch to improve that it would be great!)
> - do not use self.Xobj, use self.obj instead, so it's consistent
> among all efl. (for C object pointer)
> - consistent name of classes: epdf.Page, not epdf.EPage.
> - avoid creating dicts for things that do not need one, for example
> in EPage.text_find(), either return a tuple, or return an evas.Rect
> - watch out memory leaks! EPage.text_find() is leaking the returned
> list and nodes, you need to free it. Please review the whole code in
> parts that returns lists and char*.
> - watch out NULL -> python. If any function can return NULL on
> char*, you need to check for that and convert to None yourself,
> otherwise the application will segv.
Sorry taking sooooooo long to reply, but at least I'm doing it. I
expect this to be my last review, as code is almost good. I just need
you to double check the memory cleanup and termination of all classes.
They look wrong to me. For example:
self.obj = NULL
if self.obj == NULL:
raise ValueError("Object already deleted")
so what happens when you font.delete()? You delete self.obj, but the
pointer value is still set when __dealloc__() run, so you'll likely
get a segv on second epdf_font_info_delete(self.obj)
Also, this is never happening since you never let Python refcount to
drop to zero because you explicitly increases reference on _set_obj(),
but you never call _unset_obj() to get it down.
If you compared that to Evas_Object or its derivate you'll do a
mistake. What Evas_Object do is to call
EVAS_CALLBACK_DEL/EVAS_CALLBACK_FREE when object is deleted, so we
listen to that callback and then unset object automatically, making
the object "shallow". Also in Evas you need to incref since the
canvas itself will always keep a reference to the object, that we
release on .delete() (either from Python OR C, that's why we use the
You need to be sure of the wrapped library before wrapping it. Know
for sure who keeps references, or if everything is a copy (ie: you ask
page from document, is the page pointer kept inside document or it's a
copy that you're supposed to free directly?) After you're sure of
that, you write the code taking care of all possible use cases. For
example, if you always get a copy, then you should NOT increase
reference and no "delete" function is required, let Python's GC work
for you. If something else keeps the reference, then you need to
increment reference and you need to be notified if another peer
explicitly deleted the object (ie: in evas objects). If something else
keeps the reference and there is NO explicit delete and everything
depends on refcount, you do not need to incref and no delete() and no
callback is required.
Since nobody did epdf bindings before, it's very likely that you'll
find some cases missing code, like callbacks or so. I found lots of
them while doing initial efl bindings. In that case, go to epdf
itself, fix and provide patches. Vincent will be happy to cooperate
Vincent: could you help André to get this right?
Gustavo Sverzut Barbieri
http://profusion.mobi embedded systems
Mobile: +55 (19) 9225-2202