From 52fab72397687467650093c86e5479cb1d759042 Mon Sep 17 00:00:00 2001
From: Daniel Stenberg <daniel@haxx.se>
Date: Mon, 19 Apr 2021 10:45:29 +0200
Subject: [PATCH] checksrc: complain on == NULL or != 0 checks in conditions

... to make them all consistenly use if(!var) and if(var)

Also added a few missing warnings to the documentation.

Closes #6912
---
 docs/CHECKSRC.md | 21 ++++++++++++++++++---
 lib/checksrc.pl  | 19 ++++++++++++++++++-
 2 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/docs/CHECKSRC.md b/docs/CHECKSRC.md
index d36763bc5..2f634c49e 100644
--- a/docs/CHECKSRC.md
+++ b/docs/CHECKSRC.md
@@ -33,8 +33,9 @@ warnings are:
 - `ASSIGNWITHINCONDITION`: Assignment within a conditional expression. The
   code style mandates the assignment to be done outside of it.
 
-- `ASTERISKNOSPACE`: A pointer was declared like `char* name` instead of the more
-   appropriate `char *name` style. The asterisk should sit next to the name.
+- `ASTERISKNOSPACE`: A pointer was declared like `char* name` instead of the
+   more appropriate `char *name` style. The asterisk should sit next to the
+   name.
 
 - `ASTERISKSPACE`: A pointer was declared like `char * name` instead of the
    more appropriate `char *name` style. The asterisk should sit right next to
@@ -47,16 +48,28 @@ warnings are:
    strcat, strncat, gets are **never** allowed in curl source code.
 
 - `BRACEELSE`: '} else' on the same line. The else is supposed to be on the
-  following line.
+   following line.
 
 - `BRACEPOS`: wrong position for an open brace (`{`).
 
+- `BRACEWHILE`: more than once space between end brace and while keyword
+
 - `COMMANOSPACE`: a comma without following space
 
 - `COPYRIGHT`: the file is missing a copyright statement!
 
 - `CPPCOMMENTS`: `//` comment detected, that's not C89 compliant
 
+- `DOBRACE`: only use one space after do before open brace
+
+- `EMPTYLINEBRACE`: found empty line before open brace
+
+- `EQUALSNOSPACE`: no space after `=` sign
+
+- `EQUALSNULL`: comparison with `== NULL` used in if/while. We use `!var`.
+
+- `EXCLAMATIONSPACE`: space found after exclamations mark
+
 - `FOPENMODE`: `fopen()` needs a macro for the mode string, use it
 
 - `INDENTATION`: detected a wrong start column for code. Note that this
@@ -70,6 +83,8 @@ warnings are:
 - `NOSPACEEQUALS`: An equals sign was found without preceding space. We prefer
   `a = 2` and *not* `a=2`.
 
+- `NOTEQUALSZERO`: check found using `!= 0`. We use plain `if(var)`.
+
 - `ONELINECONDITION`: do not put the conditional block on the same line as `if()`
 
 - `OPENCOMMENT`: File ended with a comment (`/*`) still "open".
diff --git a/lib/checksrc.pl b/lib/checksrc.pl
index 13f86ecd5..a35535c19 100755
--- a/lib/checksrc.pl
+++ b/lib/checksrc.pl
@@ -6,7 +6,7 @@
 #                            | (__| |_| |  _ <| |___
 #                             \___|\___/|_| \_\_____|
 #
-# Copyright (C) 2011 - 2020, Daniel Stenberg, <daniel@haxx.se>, et al.
+# Copyright (C) 2011 - 2021, Daniel Stenberg, <daniel@haxx.se>, et al.
 #
 # This software is licensed as described in the file COPYING, which
 # you should have received as part of this distribution. The terms
@@ -86,6 +86,8 @@ my %warnings = (
     'BRACEWHILE'       => 'A single space between open brace and while',
     'EXCLAMATIONSPACE' => 'Whitespace after exclamation mark in expression',
     'EMPTYLINEBRACE'   => 'Empty line before the open brace',
+    'EQUALSNULL'       => 'if/while comparison with == NULL',
+    'NOTEQUALSZERO'    => 'if/while comparison with != 0'
     );
 
 sub readskiplist {
@@ -471,6 +473,21 @@ sub scanfile {
                           "$2 with space");
             }
         }
+        # check for '== NULL' in if/while conditions but not if the thing on
+        # the left of it is a function call
+        if($nostr =~ /^(.*)(if|while)(\(.*[^)]) == NULL/) {
+            checkwarn("EQUALSNULL", $line,
+                      length($1) + length($2) + length($3),
+                      $file, $l, "we prefer !variable instead of \"== NULL\" comparisons");
+        }
+
+        # check for '!= 0' in if/while conditions but not if the thing on
+        # the left of it is a function call
+        if($nostr =~ /^(.*)(if|while)(\(.*[^)]) != 0[^x]/) {
+            checkwarn("NOTEQUALSZERO", $line,
+                      length($1) + length($2) + length($3),
+                      $file, $l, "we prefer if(rc) instead of \"rc != 0\" comparisons");
+        }
 
         # check spaces in 'do {'
         if($nostr =~ /^( *)do( *)\{/ && length($2) != 1) {