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

diff --git a/docs/internals/CHECKSRC.md b/docs/internals/CHECKSRC.md
index ea6f260cf13b6fa7b1fe05ad83f952a9534bc265..b94adc3c072e8e1172147029194fe2ecfd1f7e26 100644 (file)
--- 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 de1d3c5e626f5322f9aa41825c26e62e5e419ace..748941d5c97cc33ee4b22d3e0109abf3081e12c9 100755 (executable)
--- 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 217a0a1604798cf23fd34ff63548da68ff865880..200a01f56539633abb39228d34879d4a2cbc0d82 100644 (file)
--- 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
 
 </file>
@@ -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
 </stdout>
 <errorcode>
 5