checksrc: introduce 'banfunc' to ban specific functions

Use 'banfunc' and 'allowfunc' in .checksrc to specify which functions to
ban or allow to be used. This saves us from having to edit the script
going forward when we want to ban or allow specific functions.

This replaces a set of previous rules and all banned functions are now
checked with the BANNEDFUNC rule.

There is a set of default banned functions, shown by invoking
./checksrc.

Also, -a and -b options are added to specify allowed or banned functions
on the command line.

Closes #15835

diff --git a/docs/examples/.checksrc b/docs/examples/.checksrc
index c7a53373943334b2189386a51024638bb6ba2871..dea90aaa1d1122d49625ab09cbcf8a5de6ad5a30 100644 (file)
--- a/docs/examples/.checksrc
+++ b/docs/examples/.checksrc
@@ -1,4 +1,3 @@
 disable TYPEDEFSTRUCT
 disable SNPRINTF
 disable BANNEDFUNC
-disable SSCANF

diff --git a/lib/.checksrc b/lib/.checksrc
index 9066946c890dd2d8a5a3fda9c82fbe2baeed8772..3985707fd9e44c1efb1d6e7f89c8b702e1f5a5a1 100644 (file)
--- a/lib/.checksrc
+++ b/lib/.checksrc
@@ -1,2 +1,5 @@
-enable STRERROR
-enable STRNCPY
+banfunc strerror
+banfunc strncpy
+banfunc sscanf
+banfunc snprintf
+banfunc vsnprint

diff --git a/lib/strerror.c b/lib/strerror.c
index 6b67a905880e0062b733efa3de5853089fb61508..32449c9fa1b4568ecfba63ae101360fe9e5a0593 100644 (file)
--- a/lib/strerror.c
+++ b/lib/strerror.c
@@ -891,7 +891,7 @@ const char *Curl_strerror(int err, char *buf, size_t buflen)
   }
 #else
   {
-    /* !checksrc! disable STRERROR 1 */
+    /* !checksrc! disable BANNEDFUNC 1 */
     const char *msg = strerror(err);
     if(msg)
       msnprintf(buf, buflen, "%s", msg);

diff --git a/scripts/checksrc.pl b/scripts/checksrc.pl
index d17f95314425234e9cfe71b9c52cf74010ba49d3..a58163e30445a5cd4eb69b2b5536ea14ccbf39a6 100755 (executable)
--- a/scripts/checksrc.pl
+++ b/scripts/checksrc.pl
@@ -47,10 +47,34 @@ my %ignore_set;
 my %ignore_used;
 my @ignore_line;
 
+my %banfunc = (
+    "gmtime" => 1,
+    "localtime" => 1,
+    "gets" => 1,
+    "strtok" => 1,
+    "sprintf" => 1,
+    "vsprintf" => 1,
+    "strcat" => 1,
+    "strncat" => 1,
+    "_mbscat" => 1,
+    "_mbsncat" => 1,
+    "_tcscat" => 1,
+    "_tcsncat" => 1,
+    "_wcscat" => 1,
+    "_wcsncat" => 1,
+    "LoadLibrary" => 1,
+    "LoadLibraryA" => 1,
+    "LoadLibraryW" => 1,
+    "LoadLibraryEx" => 1,
+    "LoadLibraryExA" => 1,
+    "LoadLibraryExW" => 1,
+    "_waccess" => 1,
+    "_access" => 1,
+    "access" => 1,
+    );
+
 my %warnings_extended = (
     'COPYRIGHTYEAR'    => 'copyright year incorrect',
-    'STRERROR',        => 'strerror() detected',
-    'STRNCPY',         => 'strncpy() detected',
     'STDERR',          => 'stderr detected',
     );
 
@@ -92,14 +116,12 @@ my %warnings = (
     'RETURNNOSPACE'         => 'return without space',
     'SEMINOSPACE'           => 'semicolon without following space',
     'SIZEOFNOPAREN'         => 'use of sizeof without parentheses',
-    'SNPRINTF'              => 'use of snprintf',
     'SPACEAFTERPAREN'       => 'space after open parenthesis',
     'SPACEBEFORECLOSE'      => 'space before a close parenthesis',
     'SPACEBEFORECOMMA'      => 'space before a comma',
     'SPACEBEFOREPAREN'      => 'space before an open parenthesis',
     'SPACESEMICOLON'        => 'space before semicolon',
     'SPACESWITCHCOLON'      => 'space before colon of switch label',
-    "SSCANF"                => 'use of sscanf',
     'TABS'                  => 'TAB characters not allowed',
     'TRAILINGSPACE'         => 'Trailing whitespace on the line',
     'TYPEDEFSTRUCT'         => 'typedefed struct',
@@ -144,14 +166,14 @@ sub readlocalfile {
         if (/^\s*(#.*)/) {
             next;
         }
-        elsif (/^\s*enable ([A-Z]+)$/) {
+        elsif (/^enable ([A-Z]+)$/) {
             if(!defined($warnings_extended{$1})) {
                 print STDERR "invalid warning specified in .checksrc: \"$1\"\n";
                 next;
             }
             $warnings{$1} = $warnings_extended{$1};
         }
-        elsif (/^\s*disable ([A-Z]+)$/) {
+        elsif (/^disable ([A-Z]+)$/) {
             if(!defined($warnings{$1})) {
                 print STDERR "invalid warning specified in .checksrc: \"$1\"\n";
                 next;
@@ -159,6 +181,12 @@ sub readlocalfile {
             # Accept-list
             push @alist, $1;
         }
+        elsif (/^banfunc ([^ ]*)/) {
+            $banfunc{$1} = $1;
+        }
+        elsif (/^allowfunc ([^ ]*)/) {
+            undef $banfunc{$1};
+        }
         else {
             die "Invalid format in $dir/.checksrc on line $i\n";
         }
@@ -223,27 +251,38 @@ $file = shift @ARGV;
 
 while(defined $file) {
 
-    if($file =~ /-D(.*)/) {
+    if($file =~ /^-D(.*)/) {
         $dir = $1;
         $file = shift @ARGV;
         next;
     }
-    elsif($file =~ /-W(.*)/) {
+    elsif($file =~ /^-W(.*)/) {
         $wlist .= " $1 ";
         $file = shift @ARGV;
         next;
     }
-    elsif($file =~ /-A(.+)/) {
+    elsif($file =~ /^-b(.*)/) {
+        $banfunc{$1} = $1;
+        print STDERR "ban use of \"$1\"\n";
+        $file = shift @ARGV;
+        next;
+    }
+    elsif($file =~ /^-a(.*)/) {
+        undef $banfunc{$1};
+        $file = shift @ARGV;
+        next;
+    }
+    elsif($file =~ /^-A(.+)/) {
         push @alist, $1;
         $file = shift @ARGV;
         next;
     }
-    elsif($file =~ /-i([1-9])/) {
+    elsif($file =~ /^-i([1-9])/) {
         $indent = $1 + 0;
         $file = shift @ARGV;
         next;
     }
-    elsif($file =~ /-m([0-9]+)/) {
+    elsif($file =~ /^-m([0-9]+)/) {
         $max_column = $1 + 0;
         $file = shift @ARGV;
         next;
@@ -260,6 +299,8 @@ if(!$file) {
     print "checksrc.pl [option] <file1> [file2] ...\n";
     print " Options:\n";
     print "  -A[rule]  Accept this violation, can be used multiple times\n";
+    print "  -a[func]  Allow use of this function\n";
+    print "  -b[func]  Ban use of this function\n";
     print "  -D[DIR]   Directory to prepend file names\n";
     print "  -h        Show help output\n";
     print "  -W[file]  Skip the given file - ignore all its flaws\n";
@@ -277,6 +318,11 @@ if(!$file) {
         }
     }
     print " [*] = disabled by default\n";
+
+    print "\nDetects and bans use of these functions:\n";
+    for my $f (sort keys %banfunc) {
+        printf (" %-18s\n", $f);
+    }
     exit;
 }
 
@@ -801,54 +847,23 @@ sub scanfile {
         }
 
         # scan for use of banned functions
-        if($l =~ /^(.*\W)
-                   (gmtime|localtime|
-                    gets|
-                    strtok|
-                    v?sprintf|
-                    (str|_mbs|_tcs|_wcs)n?cat|
-                    LoadLibrary(Ex)?(A|W)?|
-                    _?w?access)
-                   \s*\(
-                 /x) {
+        my $bl = $l;
+      again:
+        if(($l =~ /^(.*?\W)(\w+)(\s*\()/x) && $banfunc{$2}) {
+            my $bad = $2;
+            my $prefix = $1;
+            my $suff = $3;
             checkwarn("BANNEDFUNC",
-                      $line, length($1), $file, $ol,
-                      "use of $2 is banned");
-        }
-        # scan for use of sscanf. This is not a BANNEDFUNC to allow for
-        # individual enable/disable of this warning.
-        if($l =~ /^(.*\W)(sscanf)\s*\(/x) {
-            if($1 !~ /^ *\#/) {
-                # skip preprocessor lines
-                checkwarn("SSCANF",
-                          $line, length($1), $file, $ol,
-                          "use of $2 is banned");
-            }
-        }
-        if($warnings{"STRERROR"}) {
-            # scan for use of banned strerror. This is not a BANNEDFUNC to
-            # allow for individual enable/disable of this warning.
-            if($l =~ /^(.*\W)(strerror)\s*\(/x) {
-                if($1 !~ /^ *\#/) {
-                    # skip preprocessor lines
-                    checkwarn("STRERROR",
-                              $line, length($1), $file, $ol,
-                              "use of $2 is banned");
-                }
-            }
-        }
-        if($warnings{"STRNCPY"}) {
-            # scan for use of banned strncpy. This is not a BANNEDFUNC to
-            # allow for individual enable/disable of this warning.
-            if($l =~ /^(.*\W)(strncpy)\s*\(/x) {
-                if($1 !~ /^ *\#/) {
-                    # skip preprocessor lines
-                    checkwarn("STRNCPY",
-                              $line, length($1), $file, $ol,
-                              "use of $2 is banned");
-                }
-            }
-        }
+                      $line, length($prefix), $file, $ol,
+                      "use of $bad is banned");
+            my $replace = 'x' x (length($bad) + 1);
+            $prefix =~ s/\*/\\*/;
+            $suff =~ s/\(/\\(/;
+            $l =~ s/$prefix$bad$suff/$prefix$replace/;
+            goto again;
+      }
+        $l = $bl; # restore to pre-bannedfunc content
+
         if($warnings{"STDERR"}) {
             # scan for use of banned stderr. This is not a BANNEDFUNC to
             # allow for individual enable/disable of this warning.
@@ -861,12 +876,6 @@ sub scanfile {
                 }
             }
         }
-        # scan for use of snprintf for curl-internals reasons
-        if($l =~ /^(.*\W)(v?snprintf)\s*\(/x) {
-            checkwarn("SNPRINTF",
-                      $line, length($1), $file, $ol,
-                      "use of $2 is banned");
-        }
 
         # scan for use of non-binary fopen without the macro
         if($l =~ /^(.*\W)fopen\s*\([^,]*, *\"([^"]*)/) {

diff --git a/src/.checksrc b/src/.checksrc
index df9b1f0795a728fc34715b399e84208e5f56775a..272515f78f194b256bcb9750e1386a73585ab529 100644 (file)
--- a/src/.checksrc
+++ b/src/.checksrc
@@ -1,2 +1,3 @@
 enable STDERR
-enable STRNCPY
+banfunc strncpy
+banfunc sscanf

diff --git a/tests/data/test1185 b/tests/data/test1185
index daea3101c848faf0de3f380d61cef7778bdd8969..6a4d43e29136f10a96061bbc9b4ab66764add7e9 100644 (file)
--- a/tests/data/test1185
+++ b/tests/data/test1185
@@ -16,7 +16,7 @@ checksrc
 </name>
 
 <command type="perl">
-%SRCDIR/../scripts/checksrc.pl %LOGDIR/code%TESTNUMBER.c
+%SRCDIR/../scripts/checksrc.pl -bmagicbad -balsobad %LOGDIR/code%TESTNUMBER.c
 </command>
 <file name="%LOGDIR/code%TESTNUMBER.c">
 /* test source code
@@ -71,7 +71,7 @@ void startfunc(int a, int b) {
  }
 
  int a = sizeof int;
- int a = snprintf(buffer, sizeof(buffer), "%d", 99);
+ int a = magicbad(buffer, alsobad(buffer), "%d", 99);
  int moo = hej?wrong:a>b;
  int moo2 = wrong2:(a)>(b);
 
@@ -162,9 +162,12 @@ void startfunc(int a, int b) {
 ./%LOGDIR/code1185.c:52:16: warning: sizeof without parenthesis (SIZEOFNOPAREN)
   int a = sizeof int;
                 ^
-./%LOGDIR/code1185.c:53:10: warning: use of snprintf is banned (SNPRINTF)
-  int a = snprintf(buffer, sizeof(buffer), "%d", 99);
+./%LOGDIR/code1185.c:53:10: warning: use of magicbad is banned (BANNEDFUNC)
+  int a = magicbad(buffer, alsobad(buffer), "%d", 99);
           ^
+./%LOGDIR/code1185.c:53:27: warning: use of alsobad is banned (BANNEDFUNC)
+  int a = magicbad(buffer, alsobad(buffer), "%d", 99);
+                           ^
 ./%LOGDIR/code1185.c:54:21: warning: missing space before colon (NOSPACEC)
   int moo = hej?wrong:a>b;
                      ^
@@ -201,7 +204,7 @@ void startfunc(int a, int b) {
 ./%LOGDIR/code1185.c:1:1: error: Missing closing comment (OPENCOMMENT)
  
  ^
-checksrc: 0 errors and 38 warnings
+checksrc: 0 errors and 39 warnings
 </stdout>
 <errorcode>
 5

diff --git a/tests/libtest/.checksrc b/tests/libtest/.checksrc
index 3d47f3e9a70e90f6d63dde96f1540b72745a8bdf..4483855d970150fac5caa3f42f93698c2297dda0 100644 (file)
--- a/tests/libtest/.checksrc
+++ b/tests/libtest/.checksrc
@@ -1,3 +1,3 @@
 disable TYPEDEFSTRUCT
-disable BANNEDFUNC
-disable SSCANF
+allowfunc localtime
+allowfunc LoadLibrary

diff --git a/tests/libtest/stub_gssapi.c b/tests/libtest/stub_gssapi.c
index d581a91d5b7fbdae13a8fe43a654eafe1e8a513b..7fcbb641faaf73f05694dcf16dba054602ea7121 100644 (file)
--- a/tests/libtest/stub_gssapi.c
+++ b/tests/libtest/stub_gssapi.c
@@ -30,8 +30,6 @@
 
 #include "stub_gssapi.h"
 
-/* !checksrc! disable SNPRINTF all */
-
 #define MAX_CREDS_LENGTH 250
 #define APPROX_TOKEN_LEN 250
 

diff --git a/tests/server/.checksrc b/tests/server/.checksrc
index 075b9658191fdd72e3b4e1d5e7f0f6935efbf059..ac086ea4c68f389372c3e8c957938ac26dba37ef 100644 (file)
--- a/tests/server/.checksrc
+++ b/tests/server/.checksrc
@@ -1,2 +1 @@
-enable STRNCPY
-disable SSCANF
+banfunc STRNCPY