[Geany-Devel] bug hunting with cppcheck

Colomban Wendling lists.ban at xxxxx
Sat Nov 2 21:35:52 UTC 2013


Hi Pavel,

First, thanks a lot for looking at better checking the code, that's a
great thing :)

Le 02/11/2013 20:41, Pavel Roschin a écrit :
> I checked the code with cppcheck (version from git). Also I created gtk
> config for cppcheck - if you are interested in, you could help me to fill it to
> improve analysis: https://gist.github.com/scriptum/7282198
> 
> cppcheck cannot check GTK programs because glib's design is not heap-friendly:
> most functions needs freeing memory and cppcheck doesn't know about them. In
> new versions you can specify glib functions in config (there is example one),
> but this is huge work...

Why did it somewhat work in the past?  Was it luck and poor checking or
did it know about some of them?  But indeed, it couldn't really know
without a huge config file like yours.

> Geany:
> ./cppcheck ../geany --library=gtk `pkg-config --cflags glib-2.0` \
> --max-configs=1 -j32 -q --template=gcc
> [../geany/src/editor.c:4866]: (error) Memory leak: f

Fixed, thanks.

> This is the one and real (return-after-malloc) bug. g_return_if_fail is danger:
> there should be g_goto_end_if_fail and nothing other to make sure you finished
> transaction. You will find similar leak in plugins. This is _very_ dangerous
> because it doesn't look like a macro and hides return keyword from you inside!

Well, yeah, it could be misleading in existing code, but when you add it
you know it will return;  and it's quite nice when you're use to it.
But that's right that it needs to be cared about.

Also, remember that g_return*_if_fail() is supposed to check for an
*bug* in the caller code, so leaking in this case isn't IMO as much of a
big deal as if it was in normal code path.


Regards,
Colomban


More information about the Devel mailing list