Recently cppcheck started failing for G-P, e.g.: ``` ao_bookmarklist.c:93:1: warning: There is an unknown macro here somewhere. Configuration is required. If G_DEFINE_TYPE is a macro then please configure it. [unknownMacro] G_DEFINE_TYPE(AoBookmarkList, ao_bookmark_list, G_TYPE_OBJECT)
overviewprefs.c:86:1: warning: There is an unknown macro here somewhere. Configuration is required. If G_DEFINE_TYPE is a macro then please configure it. [unknownMacro] G_DEFINE_TYPE (OverviewPrefs, overview_prefs, G_TYPE_OBJECT) ```
I wonder why cppcheck doesn't find the macro anymore.
In the bugtracker and forums I did not find anything relevant: https://sourceforge.net/p/cppcheck/discussion/general https://trac.cppcheck.net/
Maybe we need to pass more include locations but then again, why now? What changed?
A full log can be found here (this is with cppcheck 2.9): https://nightly.geany.org/debian/geany-plugins_1.38-1+20220909git4d08987-1_s... (the link might break in the future, if so use the latest logfile on https://nightly.geany.org/debian/ for geany-plugins and sid)
I'm pretty sure it is cppcheck, I reproduced locally with cppcheck 2.9 while 2.8 works fine.
Does anyone has a clue? @kugel- @elextr @techee @b4n
Maybe cppcheck 2.9 needs gtk to be specified with `--library=` ? (guesses)
Good guess! It was as easy as this. Thanks. Still not sure why is this necessary now but it is easy enough, I guess.
Now, cppcheck finds new issues. The library flag and two reported findings are fixed in #1197. There is at least one more finding in `geanyctags/src/geanyctags.c` where I'm not yet sure what the problem is.
The #1197 CI shows one in debugger, see comment there.
There is at least one more finding in geanyctags/src/geanyctags.c where I'm not yet sure what the problem is.
What does the geanyctags warning say? It doesn't seem to appear in the #1197 CI log.
``` make[3]: Entering directory '/home/enrico/projects/geany-plugins/geanyctags/src' /usr/bin/cppcheck \ -q --template gcc --error-exitcode=0 \ --library=gtk \ -I/home/enrico/apps/include/geany \ \ . geanyctags.c:165:2: warning: Mismatching allocation and deallocation: argv [mismatchAllocDealloc] g_strfreev(argv); ^ geanyctags.c:125:9: note: Mismatching allocation and deallocation: argv argv = g_new0(gchar *, 4); ^ geanyctags.c:165:2: note: Mismatching allocation and deallocation: argv g_strfreev(argv); ^ ```
There are many more warnings :(. I fixed fhe most obvious ones but there are some left where I'm not sure how to fix them or whether this might be false-positives.
Here is the list of the remaining warnings: https://gist.github.com/eht16/1b126c77a28bae54d0fa8eb65b117c13
Well, for geanyctags it is a false positive in terms of bug detection but technically it really is "Mismatching allocation and deallocation".
The `argv` variable is only used on Windows but it's declared for linux builds too. The related code looks this way: ``` gchar **argv = NULL;
#ifndef G_OS_WIN32 /* run within shell so we can use pipes */ argv = g_new0(gchar *, 4); argv[0] = g_strdup("/bin/sh"); argv[1] = g_strdup("-c"); argv[2] = g_strdup(cmd); argv[3] = NULL; #endif
g_strfreev(argv); ``` Sice `g_strfreev()` on NULL pointer does nothing, there's no real problem on linux.
To eliminate the warning, the `argv` variable could be only declared/freed in the `#ifndef G_OS_WIN32` blocks because it's unused on linux.
The vimode error ``` cmd-runner.c:706:15: warning: Dereferencing 'kpl' after it is deallocated / released [deallocuse] g_free(ctx->kpl->data); ``` is a false positive though. It happens for line
https://github.com/geany/geany-plugins/blob/7cb4499878e3a88f1bf5c67b3eae03c6...
but this line is only executed when `ctx->kpl != NULL` and in the code above `ctx->kpl` is always set to NULL when `ctx->kpl` is deallocated. So there's no way this line could be executed on a deallocated `ctx->kpl`.
To eliminate the warning, the `argv` variable could be only declared/freed in the `#ifndef G_OS_WIN32` blocks because it's unused on linux.
I tried with the following changes: ```diff diff --git a/geanyctags/src/geanyctags.c b/geanyctags/src/geanyctags.c index 2cea2246..74d80002 100644 --- a/geanyctags/src/geanyctags.c +++ b/geanyctags/src/geanyctags.c @@ -111,7 +111,9 @@ static void plugin_geanyctags_help (G_GNUC_UNUSED GeanyPlugin *plugin, G_GNUC_UN static void spawn_cmd(const gchar *cmd, const gchar *dir) { GError *error = NULL; +#ifndef G_OS_WIN32 gchar **argv = NULL; +#endif gchar *working_dir; gchar *utf8_working_dir; gchar *utf8_cmd_string; @@ -162,7 +164,9 @@ static void spawn_cmd(const gchar *cmd, const gchar *dir) msgwin_msg_add(COLOR_BLACK, -1, NULL, "%s", out); }
+#ifndef G_OS_WIN32 g_strfreev(argv); +#endif g_free(working_dir); g_free(out); } ``` but it results in the same warning. I guess cppcheck ignores '#ifdef`s.
We can just suppress the warnings for this and also for the vimode warning. See "Supressions" on http://cppcheck.net/manual.html and https://github.com/geany/geany-plugins/blob/7c540bf347d0f0c6f6ab5123fa8f226b....
We can just suppress the warnings for this and also for the vimode warning.
Agree. I think all these error appear now because you added the `--library=gtk` flag - without this flag cppcheck didn't know about gtk and wasn't aware of `g_new()` or `g_free()` and took them as ordinary function calls.
I tried with the following changes:
Oh, there's a mistake - instead of `#ifndef`s there should be `#ifdef`s.
I thought `argv` is *only* used on Linux? Anyway, I tried also with `ifdef`s and it's the same result. Either my testing is wrong or cppcheck ignores these macros.
I thought argv is only used on Linux?
Ah, sorry, I just got confused, you were right.
Either my testing is wrong or cppcheck ignores these macros.
cppcheck seems to test with different combinations of preprocessor macros. See https://cppcheck.sourceforge.io/manual.html#preprocessor-settings. However, I still don't understand why it always chokes on the `argv` array. For now, I suppressed the warning and also the one in the vimmode plugin.
But there are still a few errors left where I have no idea what the problem is and how to fix them or whether we can ignore them. Any help is much appreciated.
@kugel- @techee @elextr and anyone else.
Sigh, the old cppcheck version in the CI doesn't seem to respect the inline suppressions :(. In #1201 we switched the CI system to Ubuntu 20.04. Should I cherry-pick the commit https://github.com/geany/geany-plugins/pull/1201/commits/de6304c5d63a9b59023... into here?
Maybe merge #1201 and then sort out what still fails?
However, I still don't understand why it always chokes on the argv array.
If you mean the one we talked about: ``` gchar **argv = NULL;
#ifndef G_OS_WIN32 /* run within shell so we can use pipes */ argv = g_new0(gchar *, 4); argv[0] = g_strdup("/bin/sh"); argv[1] = g_strdup("-c"); argv[2] = g_strdup(cmd); argv[3] = NULL; #endif
g_strfreev(argv); ```
then I think the problem is that cppcheck doesn't find the allocation function because in
https://github.com/danmar/cppcheck/blob/4ce76d0b58d1c26f906ad71180f9577f05f3...
`g_new0()` isn't listed as an allocation function for `g_strfreev()`. It could be added to `gtk.cfg` but I'm not sure if such a change would be accepted upstream as not all `g_new0()` calls can be deallocated with `g_strfreev()` or do by deallocating the array in the code by a loop using `g_free()` which is kind of stupid.
This is all because of the `--library=gtk` command-line parameter which on the one hand fixes the macro problem, on the other hand the gtk.cfg config file doesn't seem to be universal enough to handle all possible cases (for instance, it also complains about `g_free(some_var)` where `some_var` is `NULL` which is legal for GTK). I'm wondering if it wouldn't be better to remove `--library=gtk` (and lose some of the allocations/deallocation checks) and handle the original problem of unknown macros in some other way (not sure how though).
Maybe is there a way to provide our own cppcheck config file where we define what we need?
Why not just a suppression file?
With a big comment `// fix Glib` :smiling_imp:
Yeah, whatever works, I don't know cppcheck so I don't know all the options.
Maybe is there a way to provide our own cppcheck config file where we define what we need?
What a great idea, it's green! https://github.com/geany/geany-plugins/pull/1197/checks
Thanks for the input, replacing the GTK library configuration with a custom one with only a few macros, worked fine. I'm not sure if the Autotools part is correct or can be improved but cppcheck runs again.
I removed all the previous suppression hints.
@techee thanks for the suggestion and the great analysis above!
Closed #1196 as completed via #1197.
github-comments@lists.geany.org