Hi plugin maintainers,
In context of the "ideas on increasing quality of plugins" thread, I tested a few code checker on geany-plugins trunk, and cppcheck found a few little problems:
[./codenav/src/switch_head_impl.c:144]: (error) Memory leak: lang [./geanygdb/src/gdb-io-frame.c:186]: (error) Possible null pointer dereference: lv [./geanyinsertnum/src/insertnum.c:156]: (error) Memory leak: line_pos [./geanylatex/src/templates.c:47]: (error) Memory leak: template
I also attach here the fixes I propose for them, if you're interested.
Regards, Colomban
Am Mittwoch, den 23.02.2011, 19:32 +0100 schrieb Colomban Wendling:
[./geanygdb/src/gdb-io-frame.c:186]: (error) Possible null pointer dereference: lv
Applied. Thanks. :)
On Wed, 23 Feb 2011 19:32:07 +0100 Colomban Wendling lists.ban@herbesfolles.org wrote:
[./geanyinsertnum/src/insertnum.c:156]: (error) Memory leak: line_pos
Thanks. It was never freed actually...
On Wed, 23 Feb 2011 19:32:07 +0100 Colomban Wendling lists.ban@herbesfolles.org wrote:
[./geanylatex/src/templates.c:47]: (error) Memory leak: template
I also attach here the fixes I propose for them, if you're interested.
I'm afraid I don't see why this should be a memory leak. Can you please be so kind and go into more details to prevent similar cases in future?
Cheers, Frank
Le 26/02/2011 13:41, Frank Lanitz a écrit :
On Wed, 23 Feb 2011 19:32:07 +0100 Colomban Wendling lists.ban@herbesfolles.org wrote:
[./geanylatex/src/templates.c:47]: (error) Memory leak: template
I also attach here the fixes I propose for them, if you're interested.
I'm afraid I don't see why this should be a memory leak. Can you please be so kind and go into more details to prevent similar cases in future?
It's a possible memory leak because you exit the function conditionally after the allocation, but don't free it in this branch:
TemplateEntry *template = g_new0(TemplateEntry, 1); gchar *tmp = NULL;
/* Return if its not a searched file */ if (g_str_has_suffix(file,".gtl") == FALSE) /* Leak here! */ return;
/* ... */
g_free(template);
So the easiest fix is to allocate the memory after the conditional exit since it isn't used before.
Regards, Colomban
On Sat, 26 Feb 2011 14:21:11 +0100 Colomban Wendling lists.ban@herbesfolles.org wrote:
Le 26/02/2011 13:41, Frank Lanitz a écrit :
On Wed, 23 Feb 2011 19:32:07 +0100 Colomban Wendling lists.ban@herbesfolles.org wrote:
[./geanylatex/src/templates.c:47]: (error) Memory leak: template
I also attach here the fixes I propose for them, if you're interested.
I'm afraid I don't see why this should be a memory leak. Can you please be so kind and go into more details to prevent similar cases in future?
It's a possible memory leak because you exit the function conditionally after the allocation, but don't free it in this branch:
TemplateEntry *template = g_new0(TemplateEntry, 1); gchar *tmp = NULL;
/* Return if its not a searched file */ if (g_str_has_suffix(file,".gtl") == FALSE) /* Leak here! */ return;
/* ... */
g_free(template);
So the easiest fix is to allocate the memory after the conditional exit since it isn't used before.
True.
Ah... and now I see. You were not doing a patch against trunk of geanylatex. Its fixed already inside http://yaturl.net/2c80
Cheers, Frank
Le 26/02/2011 14:50, Frank Lanitz a écrit :
On Sat, 26 Feb 2011 14:21:11 +0100 Colomban Wendling lists.ban@herbesfolles.org wrote:
Le 26/02/2011 13:41, Frank Lanitz a écrit :
On Wed, 23 Feb 2011 19:32:07 +0100 Colomban Wendling lists.ban@herbesfolles.org wrote:
[./geanylatex/src/templates.c:47]: (error) Memory leak: template
I also attach here the fixes I propose for them, if you're interested.
I'm afraid I don't see why this should be a memory leak. Can you please be so kind and go into more details to prevent similar cases in future?
It's a possible memory leak because you exit the function conditionally after the allocation, but don't free it in this branch:
TemplateEntry *template = g_new0(TemplateEntry, 1); gchar *tmp = NULL;
/* Return if its not a searched file */ if (g_str_has_suffix(file,".gtl") == FALSE) /* Leak here! */ return;
/* ... */
g_free(template);
So the easiest fix is to allocate the memory after the conditional exit since it isn't used before.
True.
Ah... and now I see. You were not doing a patch against trunk of geanylatex. Its fixed already inside http://yaturl.net/2c80
Oops, I didn't though main development don't go in geany-plugins/geanylatex, sorry :/
However, I suggest you to do the allocation after the conditional exit rather than before and freeing it if taking the branch, because alloc/free is far from being a no-op ;)
Cheers, Colomban
On Sat, 26 Feb 2011 16:45:13 +0100 Colomban Wendling lists.ban@herbesfolles.org wrote:
Le 26/02/2011 14:50, Frank Lanitz a écrit :
On Sat, 26 Feb 2011 14:21:11 +0100 Colomban Wendling lists.ban@herbesfolles.org wrote:
Le 26/02/2011 13:41, Frank Lanitz a écrit :
On Wed, 23 Feb 2011 19:32:07 +0100 Colomban Wendling lists.ban@herbesfolles.org wrote:
[./geanylatex/src/templates.c:47]: (error) Memory leak: template
I also attach here the fixes I propose for them, if you're interested.
I'm afraid I don't see why this should be a memory leak. Can you please be so kind and go into more details to prevent similar cases in future?
It's a possible memory leak because you exit the function conditionally after the allocation, but don't free it in this branch:
TemplateEntry *template = g_new0(TemplateEntry, 1); gchar *tmp = NULL;
/* Return if its not a searched file */ if (g_str_has_suffix(file,".gtl") == FALSE) /* Leak here! */ return;
/* ... */
g_free(template);
So the easiest fix is to allocate the memory after the conditional exit since it isn't used before.
True.
Ah... and now I see. You were not doing a patch against trunk of geanylatex. Its fixed already inside http://yaturl.net/2c80
Oops, I didn't though main development don't go in geany-plugins/geanylatex, sorry :/
However, I suggest you to do the allocation after the conditional exit rather than before and freeing it if taking the branch, because alloc/free is far from being a no-op ;)
Yep, you are right. Will do this ;)
Cheers, Frank