From 419b1c0b751b257bd54787618454d90fe88e7b79 Mon Sep 17 00:00:00 2001 From: Viktor Szakats Date: Thu, 21 May 2026 00:51:04 +0200 Subject: [PATCH] checksrc: detect `curlx_safefree()` opportunities Follow-up to bcd0497c8112e05412d2c649e8d9eea2bda8020e #21700 Follow-up to 1c3289c85e1a7a939464d5c5e84382d2e250e611 #21684 Follow-up to c0f0e400e0bc43cbe8c42c6937ed0ac743a8d81a #5968 Follow-up to 0f4a03cbb6fdb84d05cb6aafe50444edad4f4119 Closes #21703 --- docs/internals/CHECKSRC.md | 4 ++++ scripts/checksrc.pl | 21 +++++++++++++++++++++ tests/data/test1185 | 9 ++++++++- 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/docs/internals/CHECKSRC.md b/docs/internals/CHECKSRC.md index ea6f260cf1..b94adc3c07 100644 --- a/docs/internals/CHECKSRC.md +++ b/docs/internals/CHECKSRC.md @@ -129,6 +129,10 @@ warnings are: - `UNUSEDIGNORE`: a `checksrc` inlined warning ignore was asked for but not used, that is an ignore that should be removed or changed to get used. +- `USESAFEFREE`: there was a `curlx_free(var)` call made right before assigning + NULL to `var`. We prefer replacing that with `curlx_safefree()`, which is + doing these two operations in a single call. + ### Extended warnings Some warnings are computationally expensive to perform, so they are turned off diff --git a/scripts/checksrc.pl b/scripts/checksrc.pl index de1d3c5e62..748941d5c9 100755 --- a/scripts/checksrc.pl +++ b/scripts/checksrc.pl @@ -201,6 +201,7 @@ my %warnings = ( 'TRAILINGSPACE' => 'Trailing whitespace on the line', 'TYPEDEFSTRUCT' => 'typedefed struct', 'UNUSEDIGNORE' => 'a warning ignore was not used', + 'USESAFEFREE' => 'replace curlx_free() + NULL assignment with curlx_safefree()', ); sub readskiplist { @@ -532,6 +533,8 @@ sub scanfile { my $l = ""; my $prep = 0; my $prevp = 0; + my $prevfreeindent = ""; + my $prevfreevar = ""; if($verbose) { printf "Checking file: $file\n"; @@ -973,6 +976,24 @@ sub scanfile { $line, length($1), $file, $ol, "no space before label"); } + if($prevfreevar ne "") { + if(rindex($l, "$prevfreeindent$prevfreevar = NULL;", 0) == 0) { + checkwarn("USESAFEFREE", + $line, length($prevfreeindent), $file, $ol, + "replace curlx_free() + NULL assignment with curlx_safefree()"); + } + } + if($l) { + if($l =~ /^( *)curlx_free\(([^)]+)\);/) { + $prevfreeindent = $1; + $prevfreevar = $2; + } + else { + $prevfreeindent = ""; + $prevfreevar = ""; + } + } + # scan for use of banned functions my $bl = $l; again: diff --git a/tests/data/test1185 b/tests/data/test1185 index 217a0a1604..200a01f565 100644 --- a/tests/data/test1185 +++ b/tests/data/test1185 @@ -91,6 +91,10 @@ void startfunc(int a, int b) { int d = impl->magicbad(1); /* member function always allowed */ int e = impl.magicbad(); /* member function always allowed */ + curlx_free(ptr); /* two line + comment */ + ptr = NULL; /* comment more */ + /* comment does not end @@ -227,13 +231,16 @@ void startfunc(int a, int b) { ./%LOGDIR/code1185.c:71:2: warning: // comment (CPPCOMMENTS) // CPP comment ? ^ +./%LOGDIR/code1185.c:78:2: warning: replace curlx_free() + NULL assignment with curlx_safefree() (USESAFEFREE) + ptr = NULL; /* more comment */ + ^ ./%LOGDIR/code1185.c:1:1: error: Missing copyright statement (COPYRIGHT) %SP ^ ./%LOGDIR/code1185.c:1:1: error: Missing closing comment (OPENCOMMENT) %SP ^ -checksrc: 3 errors and 42 warnings +checksrc: 3 errors and 43 warnings 5