From 3a663e37c2944b24fa643764d8d9811af0d2453c Mon Sep 17 00:00:00 2001 From: Andreas Dilger Date: Thu, 12 Dec 2024 16:14:36 -0700 Subject: [PATCH] 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 Change-Id: I4691d9611f752b8a58f3ed1d5e0830b24da39648 Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/57408 Tested-by: jenkins Tested-by: Maloo Reviewed-by: Timothy Day Reviewed-by: Arshad Hussain Reviewed-by: Oleg Drokin --- contrib/git-hooks/commit-msg | 40 +++++++++++++++++--------- contrib/git-hooks/tests/commit.badcoverityid | 11 +++++++ contrib/git-hooks/tests/commit.badcoveritytype | 11 +++++++ contrib/git-hooks/tests/commit.ok_coverity | 11 +++++++ libcfs/include/libcfs/libcfs_private.h | 2 +- 5 files changed, 61 insertions(+), 14 deletions(-) create mode 100644 contrib/git-hooks/tests/commit.badcoverityid create mode 100644 contrib/git-hooks/tests/commit.badcoveritytype create mode 100644 contrib/git-hooks/tests/commit.ok_coverity diff --git a/contrib/git-hooks/commit-msg b/contrib/git-hooks/commit-msg index 0ac8eb1..7864dbd 100755 --- 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 index 0000000..ee1e6ec0 --- /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 +Change-Id: I04bd207582d421cc3744e7b0f4298e738502edbe +Reviewed-by: Timothy Day +Reviewed-by: Andreas Dilger diff --git a/contrib/git-hooks/tests/commit.badcoveritytype b/contrib/git-hooks/tests/commit.badcoveritytype new file mode 100644 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 +Change-Id: I04bd207582d421cc3744e7b0f4298e738502edbe +Reviewed-by: Timothy Day +Reviewed-by: Andreas Dilger diff --git a/contrib/git-hooks/tests/commit.ok_coverity b/contrib/git-hooks/tests/commit.ok_coverity new file mode 100644 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 +Change-Id: I04bd207582d421cc3744e7b0f4298e738502edbe +Reviewed-by: Timothy Day +Reviewed-by: Andreas Dilger diff --git a/libcfs/include/libcfs/libcfs_private.h b/libcfs/include/libcfs/libcfs_private.h index 1aa0844..6ed5109 100644 --- 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 */ -- 1.8.3.1