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