Repository navigation
Fix #15172 FP constParameterPointer with mmap() addr argument - #8992
Conversation
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.
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.
|
Sorry, fix pushed.
|
dmcppcheck
left a comment
There was a problem hiding this comment.
AI review
I found no new problems. The earlier review said cfg/gnu.cfg also needed the fix, and the PR now has it. The constParameterPointer false positive on the addr argument of mmap()/mmap64() is gone with posix, bsd and gnu, and with posix,gnu and gnu,posix loaded together. uninitvar on an uninitialized pointer passed as addr is still reported.
Testing
-
Build: I built
cppcheckinpr/withmake -j8 cppcheck. It built cleanly. -
cfg tests:
bash test/cfg/runtests.sh test/cfg/posix.c,test/cfg/bsd.candtest/cfg/gnu.call printedSUCCESSand exited with 0. The script skipped its compiler syntax checks becausepkg-configis not installed here. -
Scratch test:
scratch/t.chas these functions:void f1(void *addr)callsmmap(addr, ..., MAP_FIXED, ...).void f2(char *addr)callsmmap64(addr, ...).f3passes an uninitializedchar *ptommap.f4stores themmapresult in a local.
I ran
pr/cppcheck -q --enable=style --library=X t.cwith X =posix,bsd,gnu,posix,gnuandgnu,posix. -
Results:
- No
constParameterPointeris reported forf1orf2with any of these. uninitvaronpinf3is still reported every time.- The posix leak checks (
leakReturnValNotUsed,memleak) still work.
- No
Automated review of 634e4ed by Claude. The pull request was built and tested locally.
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.