Repository navigation
Conversation
The first argument to mmap() was marked as direction="in", causing constParameterPointer to suggest declaring it as pointer to const. However, with MAP_FIXED any existing mapping at addr is replaced, so the pointed-to memory is effectively modified. Drop the direction from the first argument of mmap() in bsd.cfg and posix.cfg, and mmap64() in posix.cfg.
dmcppcheck
left a comment
There was a problem hiding this comment.
AI review
The fix is incomplete. cfg/gnu.cfg also defines mmap (around line 1989), and its arg 1 still has direction="in". When gnu.cfg is loaded, the false positive is still reported: with --library=gnu, and also with --library=posix --library=gnu and --library=gnu --library=posix. On Linux, gnu.cfg is normally loaded, so most users of this check will still get the warning. The fix needs the same one-line change in gnu.cfg and a matching test in test/cfg/gnu.c. mmap64 is fixed in every load order, since only posix.cfg defines it.
The change also removes a false uninitvar on char buf[100]; mmap(buf, ...), which is a good side effect. Uninitialized pointer arguments are still reported. I found no other problems.
Testing
I built cppcheck from both pr/ and base/.
test/cfg/runtests.sh passes for posix.c, bsd.c and gnu.c on the PR build. pkg-config is missing here, so the script skipped its compiler syntax checks.
Test file scratch/t1.c:
#include <sys/mman.h>
void f1(void *addr) {
(void)mmap(addr, 0x1000, PROT_READ, MAP_PRIVATE | MAP_FIXED, 0, 0);
}
void f2(char *addr) {
(void)mmap64(addr, 0x1000, PROT_READ, MAP_PRIVATE | MAP_FIXED, 0, 0);
}
void f4(void) {
char buf[100];
(void)mmap(buf, 0x1000, PROT_READ, MAP_PRIVATE | MAP_FIXED, 0, 0);
}
void f5(void) {
char *p;
(void)mmap(p, 0x1000, PROT_READ, MAP_PRIVATE | MAP_FIXED, 0, 0);
}Results:
--library=posixand--library=bsd: the PR fixesconstParameterPointerforf1andf2. TheconstVariable/uninitvarwarnings onbufgo away, anduninitvaronpis still reported.- gnu.cfg loaded: the false positive remains. Command and output:
$ pr/cppcheck -q --enable=style --library=gnu t1.c
t1.c:2:15: style: Parameter 'addr' can be declared as pointer to const [constParameterPointer]
t1.c:12:10: style: Variable 'buf' can be declared as const array [constVariable]
t1.c:13:16: error: Uninitialized variable: buf [uninitvar]
--library=posix --library=gnu(either order): the PR still reportsconstParameterPointerforf1(mmap), but no longer forf2(mmap64). The output forf1is the same as in base.
Automated review of 4a06233 by Claude. The pull request was built and tested locally.
| <noreturn>false</noreturn> | ||
| <leak-ignore/> | ||
| <arg nr="1" direction="in"> | ||
| <arg nr="1"> |
There was a problem hiding this comment.
AI review
cfg/gnu.cfg also defines mmap (around line 1989), and its arg 1 still has direction="in". When gnu.cfg is loaded, its definition is used, so the false positive remains. I checked --library=gnu, --library=posix --library=gnu and --library=gnu --library=posix:
#include <sys/mman.h>
void f1(void *addr) {
(void)mmap(addr, 0x1000, PROT_READ, MAP_PRIVATE | MAP_FIXED, 0, 0);
}t1.c:2:15: style: Parameter 'addr' can be declared as pointer to const [constParameterPointer]
Please make the same change in gnu.cfg and add a mmap_fixed test to test/cfg/gnu.c. runtests.sh checks that file with --library=posix,gnu.
The first argument to mmap() was marked as direction="in", causing constParameterPointer to suggest declaring it as pointer to const. However, with MAP_FIXED any existing mapping at addr is replaced, so the pointed-to memory is effectively modified. Drop the direction from the first argument of mmap() in bsd.cfg and posix.cfg, and mmap64() in posix.cfg.