[Geany-devel] geany-plugins tests failures

Colomban Wendling lists.ban at xxxxx
Sun Mar 25 18:59:46 UTC 2012


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



More information about the Devel mailing list