Hello,
While running geany-plugins tests, I hit two failures.
The first one is that cppcheck is not happy about Vala, and since multiterm is fully in Vala, it fails.
The second one is that it couldn’t detect an assignment nested in an array.
Attached two simple patches to fix these.
Cheers
On 12-03-25 08:46 AM, Quentin Glidic wrote:
Hello,
While running geany-plugins tests, I hit two failures.
The first one is that cppcheck is not happy about Vala, and since multiterm is fully in Vala, it fails.
Weird, it builds OK here with `--enable-cppcheck` and `make check`. It has a few warnings about the compiled C code but otherwise seems fine. Can you tell what the error/failure message is?
Cheers, Matthew Brush
On 25/03/2012 19:22, Matthew Brush wrote:
Weird, it builds OK here with `--enable-cppcheck` and `make check`. It has a few warnings about the compiled C code but otherwise seems fine. Can you tell what the error/failure message is?
Cheers, Matthew Brush
The debugger one: make[4]: Entering directory `/home/sardemff7/Developpement/Geany/geany-plugins/debugger/src' /usr/bin/cppcheck -q --template gcc --error-exitcode=2 . dbm_gdb.c:1483: error: Return value of allocation function g_strdup_printf is not used.
The multiterm one: make[3]: Entering directory `/home/sardemff7/Developpement/Geany/geany-plugins/multiterm/src' /usr/bin/cppcheck -q --template gcc --error-exitcode=2 . cppcheck: error: could not find or open any of the paths given.
I’m using the latest cppcheck commit.
On 12-03-25 10:33 AM, Quentin Glidic wrote:
On 25/03/2012 19:22, Matthew Brush wrote:
Weird, it builds OK here with `--enable-cppcheck` and `make check`. It has a few warnings about the compiled C code but otherwise seems fine. Can you tell what the error/failure message is?
Cheers, Matthew Brush
The debugger one: make[4]: Entering directory `/home/sardemff7/Developpement/Geany/geany-plugins/debugger/src' /usr/bin/cppcheck -q --template gcc --error-exitcode=2 . dbm_gdb.c:1483: error: Return value of allocation function g_strdup_printf is not used.
The multiterm one: make[3]: Entering directory `/home/sardemff7/Developpement/Geany/geany-plugins/multiterm/src' /usr/bin/cppcheck -q --template gcc --error-exitcode=2 . cppcheck: error: could not find or open any of the paths given.
I’m using the latest cppcheck commit.
It almost sounds like it can't see the C files compiled by Valac. Is there C files in `multiterm/src/`?
Cheers, Matthew Brush
On 25/03/2012 19:40, Matthew Brush wrote:
It almost sounds like it can't see the C files compiled by Valac. Is there C files in `multiterm/src/`?
Nothing. This plugin is disabled. If you think it should run cppcheck on Vala generated code (I’m not sure it’s worth it), at least it shouldn’t when it’s disabled.
For the debugger one, I’d guess it’s a new feature added in cppcheck and not released yet.
25.03.2012 21:51, Quentin Glidic пишет:
On 25/03/2012 19:40, Matthew Brush wrote:
It almost sounds like it can't see the C files compiled by Valac. Is there C files in `multiterm/src/`?
Nothing. This plugin is disabled. If you think it should run cppcheck on Vala generated code (I’m not sure it’s worth it), at least it shouldn’t when it’s disabled.
For the debugger one, I’d guess it’s a new feature added in cppcheck and not released yet.
actually that is causing warnings:
const char *gdb_commands[] = { g_strdup_printf("-stack-list-arguments 0 %i %i", active_frame, active_frame), "-stack-list-locals 0" }; ..... g_free((void*)gdb_commands[0]);
"Return value of allocation function g_strdup_printf is not used." heh, how dows it know? :) it's definately used later in the code, but np, I'll fix to avoid warnings, as soon as I open new pool request.
Alex
Le 25/03/2012 17:46, Quentin Glidic a écrit :
Hello,
While running geany-plugins tests, I hit two failures.
The first one is that cppcheck is not happy about Vala, and since multiterm is fully in Vala, it fails.
ACK, running cppcheck on a non-C plugin seems stupid anyway.
The second one is that it couldn’t detect an assignment nested in an array.
ACK, "initializer element is not computable at load time" which is bad C anyway.
Attached two simple patches to fix these.
Cheers
BTW, my cppcheck (1.53) finds 5 things in debugger:
dbm_gdb.c:1304: error: Possible null pointer dereference: record dbm_gdb.c:1579: error: Possible null pointer dereference: record dbm_gdb.c:1700: error: Possible null pointer dereference: record dbm_gdb.c:967: error: Memory pointed to by 'record' is freed twice. debug.c:502: error: Possible null pointer dereference: expression
The first looks like a real possible problem, if exec_sync_command() returns RC_DONE at line 618 because wait4prompt is false.
The others *may* be false positive because cppcheck guesses that if you initialize a variable to NULL you expect that passing it "by reference" may not change its value. (e.g. foo=NULL; fun(&foo); ...)
However they seem to be real problems to me here. It seems to me that exec_sync_command() can return RC_DONE either if it has or not set the command_record argument, in which case the caller can't know whether it has to free the value or if it wasn't set. So, the caller must pass a properly NULL-initialized variable; but then the caller also have to check whether that variable is non-NULL before dereferencing it -- and must free() it whatever happens.
So, IIUC:
in dbm_gdb.c on lines 1304, 1579, 1700: * record should be checked against NULL before passing it to strstr()
in dbm_gdb.c on line 1579: * record should be freed even when rc != RC_DONE (line 1578)
in dbm_gdb.c on line 967: * record should be re-initialized to NULL after the first free on line 963
in debug.c on line 502: * IIRC expression might be NULL if it happens that the W_EXPRESSION column isn't set in the model, so a NULL check should be done before passing expression to strlen() * using strlen() to check whether a string is empty seems sub-optimal too, and you could consider using the NZV() macro from geany's utils.h -- and this macro handles NULL too
That's all for now :) There are also some stuff that should be fixed for good C practices and/or C89 compliance, but I'll check these later.
Cheers, Colomban