[pduchi]

I don't have a Trac account, so there is no ticket for this; the details are below. Feel free to create one and I'll add it to the title.

#include <glib.h>

static gpointer run(gpointer data)
{
    return data;
}

void f(void)
{
    GThread *thread = g_thread_new("worker", run, NULL);
    g_thread_join(thread);
}

$ cppcheck --library=gtk thread.c
thread.c:12:1: error: Memory leak: thread [memleak]

g_thread_join() releases the thread, and gtk.cfg already lists it as a <dealloc> of g_thread_new() and g_thread_try_new(). Its <function> entry also had <leak-ignore/>, so the call was ignored and every joined thread was reported as leaked.

• <leak-ignore/> is removed from g_thread_join.
• g_thread_join, g_thread_unref, g_thread_new and g_thread_try_new get their argument and return value configuration; the last three had no <function> entry.
• The GThread group is a <resource> instead of <memory>: creating a thread has side effects, so a thread that is only created and then joined or unref'd is not unusedAllocatedMemory. A thread that is never joined is reported as resourceLeak.

Tests: test/cfg/gtk.c covers joined g_thread_new() and g_thread_try_new() threads, an unref'd thread, a GThread * parameter, a thread that is never joined (resourceLeak) and a double join (doubleFree). The false positives show without the change; cfg/gtk.cfg validates against cppcheck-cfg.rng.

[pduchi]

The cygwin failure is in Set up Cygwin, before the build; could you re-run it?

[dmcppcheck]

AI review

The PR fixes the memleak false positive, but its own example still gets two new false-positive warnings. The test file suppresses both instead of fixing them.

1. constVariablePointer / constParameterPointer on any GThread * passed to g_thread_join(). direction="in" on its argument tells cppcheck that the pointee is not modified. The real prototype is gpointer g_thread_join(GThread *thread), so following the advice and declaring const GThread * gives a discarded-qualifier warning in C and an error in C++. Dropping direction="in" removes the warning.
2. unusedAllocatedMemory on every thread that is only created and joined. The description's own example now reports "Variable 'thread' is allocated memory that is never used" instead of memleak. The cause is that the GThread group is a <memory> block, and CheckUnusedVar treats alloc+dealloc of <memory> as "unused". Creating a thread has side effects, so this is wrong. Moving the GThread group from <memory> to <resource> removes it. With that change, a thread that is never joined is still reported (as resourceLeak).

I checked a candidate fix: no direction="in" and a <resource> block. None of the false positives remain, and double join, join+unref and an ignored return value are still detected.

Testing

What I built and ran

• Built cppcheck from pr/. The PR only changes cfg/gtk.cfg and test/cfg/gtk.c, so for the baseline I ran the same binary with --library=/work/base/cfg/gtk.cfg.
• I ran test/cfg/gtk.c with the options from runtests.sh (--check-library --platform=unix64 --enable=style,information --inconclusive --force --check-level=exhaustive --inline-suppr --debug-warnings). There was no output, so it passes. It only passes because of the new inline suppressions.
• I could not compile against glib here because there are no headers. The const-qualifier mismatch follows from the documented prototype gpointer g_thread_join(GThread *thread).

Reproducing the false positives (/work/scratch/t1.c):

#include <glib.h>
static gpointer run(gpointer data) { return data; }
void f1(void) {
    GThread *thread = g_thread_new("worker", run, NULL);
    g_thread_join(thread);
}
void f6(GThread *t) {
    g_thread_join(t);
}

Command: cppcheck -q --enable=style,information --inconclusive --library=gtk --template='{line}:{id}:{message}' t1.c

With the PR:

4:constVariablePointer:Variable 'thread' can be declared as pointer to const
4:unusedAllocatedMemory:Variable 'thread' is allocated memory that is never used.
7:constParameterPointer:Parameter 't' can be declared as pointer to const

With the base gtk.cfg, the only warning in f1 is memleak (the bug the PR fixes). There is no const or unused warning.

Narrowing down the causes (edited copies of gtk.cfg):

• Removing direction="in" from g_thread_join arg 1 makes constVariablePointer and constParameterPointer disappear.
• Also turning the GThread <memory> block into <resource> makes unusedAllocatedMemory disappear.
• With both changes, the remaining warnings are the expected ones:
  • doubleFree for join twice and for join followed by g_thread_unref;
  • leakReturnValNotUsed for an ignored g_thread_new return value;
  • resourceLeak for a thread that is never joined.

---

Automated review of 5c067ab by Claude. The pull request was built and tested locally.

[dmcppcheck]

AI review

Both earlier findings are fixed in fa6fb8b. Joined, unref'd and parameter threads no longer get memleak, unusedAllocatedMemory or const warnings. Double join, join+unref and a thread that is never joined are still reported.

I found one new false positive of the same kind as before, this time on the thread's data argument. direction="in" on arg 3 of g_thread_new and g_thread_try_new makes cppcheck say that any pointer passed as data can be const. That pointer usually ends up in the thread function as a non-const gpointer. Following the advice gives a discarded-qualifier warning in C and an error in C++. Dropping direction="in" from arg 3 removes the warning, and the base config does not have it.

Earlier comments

#	Comment	By	Where	Status
1	direction="in" on g_thread_join arg causes constVariablePointer/constParameterPointer FPs	AI review	general	✅ Fixed
2	GThread group causes unusedAllocatedMemory FP for created+joined thread	AI review	general	✅ Fixed
3	direction="in" on g_thread_join arg gives const-pointer false positives	AI review	cfg/gtk.cfg:8421	✅ Fixed
4	Suppressed unusedAllocatedMemory/constVariablePointer in test are false positives	AI review	test/cfg/gtk.c:741	✅ Fixed

Testing

Build and the PR's test

• Built cppcheck from pr/.
• Ran test/cfg/gtk.c with the runtests.sh options (--check-library --platform=unix64 --enable=style,information --inconclusive --force --check-level=exhaustive --inline-suppr --debug-warnings --library=gtk). The only output was the checkers report, so the test passes.

Variations (t.c) with data passed to the thread, g_thread_try_new with a GError**, returning the result of g_thread_join, g_thread_ref+unref+join, a thread stored in a struct, and join followed by unref:

• None of the earlier memleak false positives remain.
• Join followed by unref still gives doubleFree.
• A memleak for a malloc'd data pointer that is handed to the thread also appears with the base gtk.cfg, so the PR did not cause it.

New false positive (/work/scratch/t2.c):

#include <glib.h>
static gpointer run(gpointer data) { *(int*)data = 1; return NULL; }
void f2(int *p) {
    GThread *t = g_thread_new("w", run, p);
    g_thread_join(t);
}
void f3(int *p) {
    GThread *t = g_thread_try_new("w", run, p, NULL);
    if (t) g_thread_join(t);
}
void f4(void) {
    int x = 0;
    int *p = &x;
    GThread *t = g_thread_new("w", run, p);
    g_thread_join(t);
}

Command: cppcheck -q --enable=style --inconclusive --library=gtk --template='{line}:{id}:{message}' t2.c

With the PR:

3:constParameterPointer:Parameter 'p' can be declared as pointer to const
7:constParameterPointer:Parameter 'p' can be declared as pointer to const
13:constVariablePointer:Variable 'p' can be declared as pointer to const

• With the base gtk.cfg there are no const warnings, only the old memleak: t.
• With a copy of the PR's gtk.cfg where <arg nr="3" direction="in"/> is changed to <arg nr="3"/> for both functions, there is no output at all.

---

Automated review of fa6fb8b by Claude. The pull request was built and tested locally.

[dmcppcheck]

AI review

direction="in" on data (a gpointer) tells cppcheck that the pointee is not modified. The pointer is handed to the thread function, which gets it as a non-const gpointer and usually writes through it. As a result, every pointer passed here gets constParameterPointer or constVariablePointer:

static gpointer run(gpointer data) { *(int*)data = 1; return NULL; }
void f2(int *p) { GThread *t = g_thread_new("w", run, p); g_thread_join(t); }
// 1:constParameterPointer:Parameter 'p' can be declared as pointer to const

Following that advice gives a discarded-qualifier warning in C and an error in C++. The base config does not give this warning. Using <arg nr="3"/> removes it. The same applies to g_thread_try_new at line 8414.

[dmcppcheck]

AI review

Same issue as g_thread_new arg 3: direction="in" on the gpointer data argument causes constParameterPointer or constVariablePointer for pointers passed to the thread, which the thread usually modifies. <arg nr="3"/> removes the warning.

[dmcppcheck]

This pull request has merge conflicts that must be resolved.

[pduchi]

This pull request has merge conflicts that must be resolved.

Fixed

[dmcppcheck]

AI review

I found no new problems. The data const false positive from the last review (C8/C9) is fixed: arg 3 of g_thread_new and g_thread_try_new no longer has direction="in". The fixes from the earlier reviews are still in place: g_thread_join has no direction="in", and GThread is a <resource>. A thread that is joined, unref'd or passed in as a parameter gets no memleak, unusedAllocatedMemory or const warning. Double join, double unref, an ignored return value, a thread that is never joined, and an unchecked g_thread_try_new result passed to g_thread_join are still reported.

Earlier comments

#	Comment	By	Where	Status
1	direction="in" on g_thread_join arg causes constVariablePointer/constParameterPointer false positives	AI review	general	✅ Fixed
2	GThread group causes unusedAllocatedMemory false positive for created+joined thread	AI review	general	✅ Fixed
3	direction="in" on g_thread_join arg gives const-pointer false positives	AI review	cfg/gtk.cfg:8421	✅ Fixed
4	Suppressed unusedAllocatedMemory/constVariablePointer in test are false positives	AI review	test/cfg/gtk.c:741	✅ Fixed
5	direction="in" on g_thread_new data arg causes const-pointer false positives	AI review	cfg/gtk.cfg:8400	✅ Fixed
6	direction="in" on g_thread_try_new data arg causes const-pointer false positives	AI review	cfg/gtk.cfg:8414	✅ Fixed

Testing

• I built cppcheck from pr/. To get the baseline, I ran the same binary with --library=/work/base/cfg/gtk.cfg.
• I ran test/cfg/gtk.c with the runtests.sh options (--check-library --platform=unix64 --enable=style,information --inconclusive --force --check-level=exhaustive --inline-suppr --debug-warnings --library=gtk). There was no output apart from the checkers report, so the test passes.
• I could not validate against the RNG because xmllint is not installed. cppcheck loads the cfg without errors.
• I wrote variations in scratch/t.c and scratch/t.cpp:
  • g_thread_ref + g_thread_unref + g_thread_join
  • returning the result of g_thread_join
  • returning the thread
  • storing the thread in a struct parameter and in a local struct
  • g_thread_try_new with a GError*
  • double unref
  • ref after join
  • discarded g_thread_new
  • uninitialized and NULL arguments to g_thread_join
  • a thread stored in an array
  • an unchecked g_thread_try_new result passed to g_thread_join
  • C++ classes with and without a destructor
• Results with the PR:
  • The base false positives are gone: memleak at lines 26 and 59 and unusedAllocatedMemory at line 28.
  • These are still reported: doubleFree (double unref), leakReturnValNotUsed, resourceLeak: s.t, uninitvar and nullPointer.
  • There is one new and correct warning: nullPointerOutOfResources when the result of g_thread_try_new is passed to g_thread_join without a check.
  • The same cases are not detected with or without the PR: use of a thread after join, and a thread stored in a struct parameter.
• I found no new false positives.

---

Automated review of feffd1d by Claude. The pull request was built and tested locally.