Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cppcheck throws error - warning: There is an unknown macro here somewhere G_DEFINE_TYPE #1196

Closed
eht16 opened this issue Sep 11, 2022 · 21 comments · Fixed by #1197
Closed

cppcheck throws error - warning: There is an unknown macro here somewhere G_DEFINE_TYPE #1196

eht16 opened this issue Sep 11, 2022 · 21 comments · Fixed by #1197
Labels
build system Automake build system issues

Comments

@eht16
Copy link
Member

eht16 commented Sep 11, 2022

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_sid-amd64.log
(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

@eht16 eht16 added the build system Automake build system issues label Sep 11, 2022
@elextr
Copy link
Member

elextr commented Sep 12, 2022

Maybe cppcheck 2.9 needs gtk to be specified with --library= ? (guesses)

eht16 added a commit to eht16/geany-plugins that referenced this issue Sep 12, 2022
Recent versions of cppcheck (2.9+) seem to require this configuration.
The gtk.cfg configuration file is part of cppcheck itself.

Closes geany#1196.
@eht16
Copy link
Member Author

eht16 commented Sep 12, 2022

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.

@elextr
Copy link
Member

elextr commented Sep 12, 2022

The #1197 CI shows one in debugger, see comment there.

@techee
Copy link
Member

techee commented Sep 13, 2022

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.

@eht16
Copy link
Member Author

eht16 commented Sep 13, 2022

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);
 ^

@eht16
Copy link
Member Author

eht16 commented Sep 13, 2022

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

@techee
Copy link
Member

techee commented Sep 13, 2022

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.

@techee
Copy link
Member

techee commented Sep 13, 2022

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

g_free(ctx->kpl->data);

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.

@eht16
Copy link
Member Author

eht16 commented Sep 13, 2022

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 --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

AM_CPPCHECKFLAGS = --suppress='constStatement:*'
.

@techee
Copy link
Member

techee commented Sep 13, 2022

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.

@techee
Copy link
Member

techee commented Sep 13, 2022

I tried with the following changes:

Oh, there's a mistake - instead of #ifndefs there should be #ifdefs.

@eht16
Copy link
Member Author

eht16 commented Sep 14, 2022

I thought argv is only used on Linux?
Anyway, I tried also with ifdefs and it's the same result. Either my testing is wrong or cppcheck ignores these macros.

@techee
Copy link
Member

techee commented Sep 14, 2022

I thought argv is only used on Linux?

Ah, sorry, I just got confused, you were right.

@eht16
Copy link
Member Author

eht16 commented Nov 13, 2022

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.

@eht16
Copy link
Member Author

eht16 commented Nov 13, 2022

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 de6304c into here?

@elextr
Copy link
Member

elextr commented Nov 14, 2022

Maybe merge #1201 and then sort out what still fails?

@techee
Copy link
Member

techee commented Nov 14, 2022

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/4ce76d0b58d1c26f906ad71180f9577f05f3642b/cfg/gtk.cfg#L702

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).

@techee
Copy link
Member

techee commented Nov 14, 2022

Maybe is there a way to provide our own cppcheck config file where we define what we need?

@elextr
Copy link
Member

elextr commented Nov 15, 2022

Why not just a suppression file?

With a big comment // fix Glib 😈

@techee
Copy link
Member

techee commented Nov 15, 2022

Yeah, whatever works, I don't know cppcheck so I don't know all the options.

eht16 added a commit to eht16/geany-plugins that referenced this issue Nov 15, 2022
Recent versions of cppcheck (2.9+) seem to require this configuration.
The gtk.cfg configuration file is part of cppcheck itself.

Closes geany#1196.
@eht16
Copy link
Member Author

eht16 commented Nov 15, 2022

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!

eht16 added a commit to eht16/geany-plugins that referenced this issue Nov 28, 2022
Also fix a couple of new findings with current cppcheck version.

Closes geany#1196.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build system Automake build system issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants