LU-17000 libcfs: quiet Coverity LIBCFS_ALLOC warning

Coverity has a frequent but harmeless warning in LIBCFS_ALLOC_PRE():

    logical_vs_bitwise: The expression
        cptab->ctb_nparts * 4UL /* sizeof (*part->cpt_distance) */ <=
	(8192UL /* 2 << 12 */) && 1
	  /* ((gfp_t)(0x200000U | 0x400000U) | (gfp_t)64U) &
	     (((gfp_t)32U | (gfp_t)524288U) | (gfp_t)4194304U) */
    is suspicious because it performs a Boolean operation on a
    constant other than 0 or 1.

Fix by comparing GFP mask to zero, rather than the whole expression.

There are many similar CIDs, only listing a couple for reference.

Add a Git commit-msg hook check for CoverityID: in commit messages,
and test cases for the same.

Test-Parameters: trivial
CoverityID: 397137 ("Logical vs. bitwise operator")
CoverityID: 397837 ("Logical vs. bitwise operator")
Fixes: 3a92c850b0 ("LU-56 libcfs: NUMA allocator and code cleanup")
Signed-off-by: Andreas Dilger <adilger@whamcloud.com>
Change-Id: I4691d9611f752b8a58f3ed1d5e0830b24da39648
Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/57408
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Timothy Day <timday@amazon.com>
Reviewed-by: Arshad Hussain <arshad.hussain@aeoncomputing.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

diff --git a/contrib/git-hooks/commit-msg b/contrib/git-hooks/commit-msg
index 0ac8eb1..7864dbd 100755 (executable)
--- a/contrib/git-hooks/commit-msg
+++ b/contrib/git-hooks/commit-msg
@@ -17,15 +17,16 @@ init() {
        readonly ORIGINAL="$1"
        readonly REVISED="$(mktemp "$ORIGINAL.XXXXXX")"
        readonly SAVE="$(basename $ORIGINAL).$(date +%Y%m%d.%H%M%S)"
-       readonly SIGNOFF="Signed-off-by:"
+       readonly BUILD_PARAMS="Build-Parameters:"
        readonly CHANGEID="Change-Id:"
+       readonly COVERITY="CoverityID:"
        readonly FIXES="Fixes:"
-       readonly BUILD_PARAMS="Build-Parameters:"
-       readonly TEST_PARAMS="Test-Parameters:"
-       readonly TEST_PARAMS2="Test-parameters:"
+       readonly LINUX_COMMIT="Linux-commit:"
        readonly LUSTRE_CHANGE="Lustre-change:"
        readonly LUSTRE_COMMIT="Lustre-commit:"
-       readonly LINUX_COMMIT="Linux-commit:"
+       readonly SIGNOFF="Signed-off-by:"
+       readonly TEST_PARAMS="Test-Parameters:"
+       readonly TEST_PARAMS2="Test-parameters:"
        readonly EMAILS=$(echo \
                        Acked-by \
                        Tested-by \
@@ -126,6 +127,15 @@ function do_buildparams() {
                error "only {client,server}{distro,arch}= supported"
 }
 
+function do_coverity() {
+       local cid=$(awk '{ print $2 }' <<<$LINE)
+
+       [[ x${cid}x =~ x[0-9]{6,7}x ]] ||
+               error "invalid $COVERITY CID, want 'nncidnn (\"issue type\")'"
+       [[ "$LINE" =~ \ \(\".*\"\) ]] ||
+               error "invalid $COVERITY type, want 'nncidnn (\"issue type\")'"
+}
+
 function do_testparams() {
        ck_wrapup
 
@@ -137,8 +147,11 @@ function do_fixes() {
        ck_wrapup
 
        local commit=$(awk '{ print $2 }' <<<$LINE)
+
        git describe --tags $commit 2>&1 | grep "[Nn]ot a valid" &&
-               error "invalid $FIXES hash, want '<12hex> (\"summary line\")'"
+               error "invalid $FIXES hash, want '<10hex> (\"summary line\")'"
+       [[ "$LINE" =~ \ \(\".*\"\) ]] ||
+               error "invalid $FIXES summary, want '<10hex> (\"summary line\")'"
 }
 
 # All "emails" lines specify a person and email address
@@ -151,7 +164,7 @@ function do_emails() {
 
 # All "change" lines specify a Gerrit URL
 #
-function do_change() {
+function do_lustre_change() {
        local url="${LINE#*change: }"
 
        ck_is_ascii
@@ -162,7 +175,7 @@ function do_change() {
 # All "commit" lines specify a commit hash, but the commit may be in
 # another repo, so the hash can't be directly verified
 #
-function do_commit() {
+function do_lustre_commit() {
        local val=${LINE#*commit: }
 
        ck_is_ascii
@@ -301,15 +314,16 @@ exec 3< "$ORIGINAL" 4> "$REVISED" || exit 1
 while IFS= read -u3 LINE; do
        ((NUM += 1))
        case "$LINE" in
-       $SIGNOFF* )             do_signoff ;;
+       $BUILD_PARAMS* )        do_buildparams ;;
        $CHANGEID* )            do_changeid ;;
+       $COVERITY* )            do_coverity ;;
        $FIXES* )               do_fixes ;;
-       $BUILD_PARAMS* )        do_buildparams ;;
+       $LINUX_COMMIT* )        do_lustre_commit ;;
+       $LUSTRE_CHANGE* )       do_lustre_change ;;
+       $LUSTRE_COMMIT* )       do_lustre_commit ;;
+       $SIGNOFF* )             do_signoff ;;
        $TEST_PARAMS* )         do_testparams ;;
        $TEST_PARAMS2* )        do_testparams ;;
-       $LUSTRE_CHANGE* )       do_change ;;
-       $LUSTRE_COMMIT* )       do_commit ;;
-       $LINUX_COMMIT* )        do_commit ;;
 
        "")
                HAS_LAST_BLANK=true

diff --git a/contrib/git-hooks/tests/commit.badcoverityid b/contrib/git-hooks/tests/commit.badcoverityid
new file mode 100644 (file)
index 0000000..ee1e6ec
--- /dev/null
+++ b/contrib/git-hooks/tests/commit.badcoverityid
@@ -0,0 +1,11 @@
+LU-17000 utils: Fix check after return from fopen()
+
+Return from fopen() for option 't' (trace) was checked
+with a different variable. This patch fixes this.
+
+Test-Parameters: trivial
+CoverityID: ("Copy-paste error")
+Signed-off-by: Arshad Hussain <arshad.hussain@aeoncomputing.com>
+Change-Id: I04bd207582d421cc3744e7b0f4298e738502edbe
+Reviewed-by: Timothy Day <timday@amazon.com>
+Reviewed-by: Andreas Dilger <adilger@whamcloud.com>

diff --git a/contrib/git-hooks/tests/commit.badcoveritytype b/contrib/git-hooks/tests/commit.badcoveritytype
new file mode 100644 (file)
index 0000000..d86f5e2
--- /dev/null
+++ b/contrib/git-hooks/tests/commit.badcoveritytype
@@ -0,0 +1,11 @@
+LU-17000 utils: Fix check after return from fopen()
+
+Return from fopen() for option 't' (trace) was checked
+with a different variable. This patch fixes this.
+
+Test-Parameters: trivial
+CoverityID: 123445
+Signed-off-by: Arshad Hussain <arshad.hussain@aeoncomputing.com>
+Change-Id: I04bd207582d421cc3744e7b0f4298e738502edbe
+Reviewed-by: Timothy Day <timday@amazon.com>
+Reviewed-by: Andreas Dilger <adilger@whamcloud.com>

diff --git a/contrib/git-hooks/tests/commit.ok_coverity b/contrib/git-hooks/tests/commit.ok_coverity
new file mode 100644 (file)
index 0000000..47f7c55
--- /dev/null
+++ b/contrib/git-hooks/tests/commit.ok_coverity
@@ -0,0 +1,11 @@
+LU-17000 utils: Fix check after return from fopen()
+
+Return from fopen() for option 't' (trace) was checked
+with a different variable. This patch fixes this.
+
+Test-Parameters: trivial
+CoverityID: 397651 ("Copy-paste error")
+Signed-off-by: Arshad Hussain <arshad.hussain@aeoncomputing.com>
+Change-Id: I04bd207582d421cc3744e7b0f4298e738502edbe
+Reviewed-by: Timothy Day <timday@amazon.com>
+Reviewed-by: Andreas Dilger <adilger@whamcloud.com>

diff --git a/libcfs/include/libcfs/libcfs_private.h b/libcfs/include/libcfs/libcfs_private.h
index 1aa0844..6ed5109 100644 (file)
--- a/libcfs/include/libcfs/libcfs_private.h
+++ b/libcfs/include/libcfs/libcfs_private.h
@@ -137,7 +137,7 @@ do {                                                \
 do {                                                                       \
        LASSERT(!in_interrupt() ||                                          \
                (((size) <= LIBCFS_VMALLOC_SIZE) &&                         \
-                ((mask) & GFP_ATOMIC)) != 0);                              \
+                ((mask) & GFP_ATOMIC) != 0));                              \
 } while (0)
 
 /* message format here needs to match regexp in lustre/tests/leak_finder.pl */