[Geany-devel] geany-plugins tests failures
lists.ban at xxxxx
Sun Mar 25 18:59:46 UTC 2012
Le 25/03/2012 17:46, Quentin Glidic a écrit :
> 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
> Attached two simple patches to fix these.
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.
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.
More information about the Devel