GNOME Bugzilla – Bug 777833
tools: Fix an incorrect g_new() call
Last modified: 2017-08-08 18:39:30 UTC
One patch attached to do that, and a couple of other patches to fix some memory leaks I spotted while testing.
Created attachment 344420 [details] [review] tools: Fix an incorrect g_new() call It was allocating an array of guint8 pointers, rather than an array of guint8s. This resulted in an allocation 4 times too large, which is a bug, but not one which would cause crashes. Coverity ID: 1398303
Created attachment 344421 [details] [review] tools: Fix a memory leak by not closing the handles RsvgHandle needs to be explicitly closed before the last reference is dropped to it. Ideally, that would be fixed by making RsvgHandle a normal GObject which conforms to established practices. But for now, call rsvg_handle_close().
Created attachment 344422 [details] [review] tools: Fix a minor memory leak of command line arguments
Federico, ping?
Review of attachment 344420 [details] [review]: Looks fine!
Review of attachment 344421 [details] [review]: Ooh. Indeed, it looks like we leak some stuff if the handle is disposed without rsvg_handle_close_impl() having been called first. Unfortunately this is not the correct fix. Some refactoring is in order; there is some duplicated code to deal with resource management. See for example: * rsvg-base.c:rsvg_handle_close() may call rsvg_handle_read_stream_sync(); that one takes care of freeing the priv->ctxt and the XML doc. * Alternatively, it will call rsvg_handle_close_impl(); that one frees the priv->ctxt and the XML doc. It *also* calls free_element_name_stack(), while the previous one doesn't do that. * rsvg-gobject.c:rsvg_handle_dispose() should just do the freeing above if the handle is unref()ed for the last time before it is done reading. Would you like to take care of these? I can do it if you don't have time.
Review of attachment 344422 [details] [review]: Looks good!
Pushed the two a-c_n patches. Would you mind doing the updates to the other one? I’m quite short on time at the moment. Thanks! Attachment 344420 [details] pushed as 63f008b - tools: Fix an incorrect g_new() call Attachment 344422 [details] pushed as 717595e - tools: Fix a minor memory leak of command line arguments
(In reply to Philip Withnall from comment #8) > Pushed the two a-c_n patches. Would you mind doing the updates to the other > one? I’m quite short on time at the moment. Thanks! I'll do that. Thanks for pushing the other ones.
Done! See the string of commits up to b617c1a9407e29a26a4965554a51ef3fd81193e5 in the librsvg-2.40 branch, or up to 31b6af3ce820d4f1b652ea72f8a0d9476c1f2937 in master.