Added ShellCheck processing and reviewed cppcheck

yahoojapan · Dec 14, 2022 · 2053ad0 · 2053ad0
1 parent c43c661
commit 2053ad0
Show file tree

Hide file tree

Showing 14 changed files with 1,190 additions and 897 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -141,7 +141,11 @@ jobs:
           /bin/sh -c "$GITHUB_WORKSPACE/.github/workflows/build_helper.sh -os ${{ matrix.container }}"
 
   dockerimage:
-    runs-on: ubuntu-latest
+    # [NOTE]
+    # We plan to update to ubuntu-latest (22.04) soon.
+    # Right now I am forced to use 20.04 as there are no relevant packages yet.
+    #
+    runs-on: ubuntu-20.04
 
     #
     # build matrix for containers

diff --git a/.github/workflows/docker_helper.sh b/.github/workflows/docker_helper.sh
@@ -777,21 +777,19 @@ fi
 #
 # Create dockerfile from template(Common conversion)
 #
-sed -e "s#%%GIT_DOMAIN_URL%%#${CI_GIT_URL}#g"							\
-	-e "s#%%DOCKER_IMAGE_BASE%%#${CI_DOCKER_IMAGE_BASE}#g"				\
-	-e "s#%%DOCKER_IMAGE_DEV_BASE%%#${CI_DOCKER_IMAGE_DEV_BASE}#g"		\
-	-e "s#%%DOCKER_GIT_ORGANIZATION%%#${CI_DOCKER_GIT_ORGANIZATION}#g"	\
-	-e "s#%%DOCKER_GIT_REPOSITORY%%#${CI_DOCKER_GIT_REPOSITORY}#g"		\
-	-e "s#%%DOCKER_GIT_BRANCH%%#${CI_DOCKER_GIT_BRANCH}#g"				\
-	-e "s#%%PKG_UPDATE%%#${PKGMGR_NAME} ${PKGMGR_UPDATE_OPT}#g"			\
-	-e "s#%%PKG_INSTALL_BUILDER%%#${PKG_INSTALL_BUILDER_COMMAND}#g"		\
-	-e "s#%%PKG_INSTALL_BIN%%#${PKG_INSTALL_BIN_COMMAND}#g"				\
-	-e "s#%%UPDATE_LIBPATH%%#${UPDATE_LIBPATH}#g"						\
-	-e "s#%%BUILD_ENV%%#${BUILDER_ENVIRONMENT}#g"						\
-	"${SRCTOP}/${DOCKERFILE_TEMPL_SUBDIR}/${DOCKER_TEMPL_FILE}"			> "${SRCTOP}/${DOCKER_FILE}"
-
-# shellcheck disable=SC2181
-if [ $? -ne 0 ]; then
+if ! sed -e "s#%%GIT_DOMAIN_URL%%#${CI_GIT_URL}#g"							\
+		-e "s#%%DOCKER_IMAGE_BASE%%#${CI_DOCKER_IMAGE_BASE}#g"				\
+		-e "s#%%DOCKER_IMAGE_DEV_BASE%%#${CI_DOCKER_IMAGE_DEV_BASE}#g"		\
+		-e "s#%%DOCKER_GIT_ORGANIZATION%%#${CI_DOCKER_GIT_ORGANIZATION}#g"	\
+		-e "s#%%DOCKER_GIT_REPOSITORY%%#${CI_DOCKER_GIT_REPOSITORY}#g"		\
+		-e "s#%%DOCKER_GIT_BRANCH%%#${CI_DOCKER_GIT_BRANCH}#g"				\
+		-e "s#%%PKG_UPDATE%%#${PKGMGR_NAME} ${PKGMGR_UPDATE_OPT}#g"			\
+		-e "s#%%PKG_INSTALL_BUILDER%%#${PKG_INSTALL_BUILDER_COMMAND}#g"		\
+		-e "s#%%PKG_INSTALL_BIN%%#${PKG_INSTALL_BIN_COMMAND}#g"				\
+		-e "s#%%UPDATE_LIBPATH%%#${UPDATE_LIBPATH}#g"						\
+		-e "s#%%BUILD_ENV%%#${BUILDER_ENVIRONMENT}#g"						\
+		"${SRCTOP}/${DOCKERFILE_TEMPL_SUBDIR}/${DOCKER_TEMPL_FILE}"			> "${SRCTOP}/${DOCKER_FILE}"; then
+
 	PRNERR "Failed to creating ${DOCKER_FILE} from ${DOCKER_TEMPL_FILE} (Common conversion)."
 	exit 1
 fi

diff --git a/.github/workflows/imagetypevars.sh b/.github/workflows/imagetypevars.sh
@@ -1,3 +1,4 @@
+
 #
 # k2htpdtor for K2HASH TRANSACTION PLUGIN.
 #
@@ -213,12 +214,10 @@ custom_dockerfile_conversion()
 	fi
 	_TMP_DOCKERFILE_PATH="$1"
 
-	sed -e "s#%%CONFIGURE_FLAG%%#${BUILDER_CONFIGURE_FLAG}#g"	\
-		-e "s#%%BUILD_FLAGS%%#${BUILDER_MAKE_FLAGS}#g"			\
-		-i "${_TMP_DOCKERFILE_PATH}"
+	if ! sed -e "s#%%CONFIGURE_FLAG%%#${BUILDER_CONFIGURE_FLAG}#g"	\
+			-e "s#%%BUILD_FLAGS%%#${BUILDER_MAKE_FLAGS}#g"			\
+			-i "${_TMP_DOCKERFILE_PATH}"; then
 
-	# shellcheck disable=SC2181
-	if [ $? -ne 0 ]; then
 		PRNERR "Failed to converting ${_TMP_DOCKERFILE_PATH}"
 		return 1
 	fi

diff --git a/.github/workflows/ostypevars.sh b/.github/workflows/ostypevars.sh
@@ -273,7 +273,6 @@ fi
 #	RUN_POST_PACKAGE		0
 #	RUN_PUBLISH_PACKAGE		1
 #
-RUN_SHELLCHECK=0
 
 #---------------------------------------------------------------
 # Variables for each process

diff --git a/Makefile.am b/Makefile.am
@@ -15,20 +15,83 @@
 #
 
 SUBDIRS=lib src docs tests buildutils
+
 EXTRA_DIST=RELEASE_VERSION @CONFIGURECUSTOM@
 
-CPPCHECK_CMD=	cppcheck
-CPPCHECK_OPT=	--quiet \
-				--error-exitcode=1 \
-				--inline-suppr \
-				-j 4 \
-				--std=c++03 \
-				--xml \
-				--enable=warning,style,information,missingInclude
-CPPCHECK_IGN=
+.PHONY: cppcheck shellcheck
+
+#
+# CppCheck
+#
+# [NOTE]
+# Versions of cppcheck below 1.8 perform poorly and the test is
+# skipped if those versions are installed.
+#
+CPPCHECK_NG_VERSION		= -e \^0\\. -e \^1\\.[0-7]
+CPPCHECK_CMD			= cppcheck
+CPPCHECK_TARGET			= $(SUBDIRS)
+CPPCHECK_BUILD_DIR		= /tmp/cppcheck
+CPPCHECK_BASE_OPT		= --quiet \
+						  --error-exitcode=1 \
+						  --inline-suppr \
+						  -j 4 \
+						  --std=c++03 \
+						  --xml
+CPPCHECK_ENABLE_OPT		= --enable=warning,style,information,missingInclude
+CPPCHECK_IGNORE_OPT		= --suppress=unmatchedSuppression
+CPPCHECK_DEFINE_OPT		=
 
 cppcheck:
-	$(CPPCHECK_CMD) $(CPPCHECK_OPT) $(CPPCHECK_IGN) $(SUBDIRS)
+	@if command -v $(CPPCHECK_CMD) >/dev/null 2>&1; then \
+		if ($(CPPCHECK_CMD) --version | sed -e 's|Cppcheck[[:space:]]*||gi' | grep -q $(CPPCHECK_NG_VERSION)); then \
+			echo "*** [INFO] cppcheck version below 1.8, so skip to run cppcheck."; \
+		else \
+			echo "*** Check all files with CppCheck"; \
+			if [ -d $(CPPCHECK_BUILD_DIR) ]; then \
+				rm -rf $(CPPCHECK_BUILD_DIR); \
+			fi; \
+			mkdir -p $(CPPCHECK_BUILD_DIR); \
+			$(CPPCHECK_CMD) $(CPPCHECK_BASE_OPT) $(CPPCHECK_DEFINE_OPT) $(CPPCHECK_ENABLE_OPT) $(CPPCHECK_IGNORE_OPT) --cppcheck-build-dir=$(CPPCHECK_BUILD_DIR) $(CPPCHECK_TARGET); \
+			rm -rf $(CPPCHECK_BUILD_DIR); \
+		fi; \
+	else \
+		echo "*** [WARNING] cppcheck is not installed, so skip to run cppcheck."; \
+	fi
+
+#
+# ShellCheck
+#
+# This make target executes shellchek on text files with the
+# extension '*.sh' under the current directory(other than the
+# '.git' directory) and executable files(text files other than
+# the extension '*.sh').
+#
+SHELLCHECK_CMD				= shellcheck
+SHELLCHECK_BASE_OPT			= --shell=sh
+SHELLCHECK_IGN_OPT			= --exclude=SC1117,SC1090,SC1091
+SHELLCHECK_INCLUDE_IGN_OPT	= --exclude=SC1117,SC1090,SC1091,SC2034,SC2148
+SHELLCHECK_FILES_NO_SH		= `grep -ril '^\#!/bin/sh' . | grep -v '\.sh' | grep -v '\.log' | grep -v '/\.git/' | grep -v '/rpmbuild/' | grep -v '/debian_build/' | grep -v '/autom4te.cache/' | grep -v '/m4/' | grep -v '/install-sh' | grep -v '/ltmain.sh' | tr '\n' ' '`
+SHELLCHECK_FILES_SH			= `grep -ril '^\#!/bin/sh' . | grep '\.sh'    | grep -v '\.log' | grep -v '/\.git/' | grep -v '/rpmbuild/' | grep -v '/debian_build/' | grep -v '/autom4te.cache/' | grep -v '/m4/' | grep -v '/install-sh' | grep -v '/ltmain.sh' | tr '\n' ' '`
+SHELLCHECK_FILES_INCLUDE_SH	= `grep -Lir '^\#!/bin/sh' . | grep '\.sh'    | grep -v '\.log' | grep -v '/\.git/' | grep -v '/rpmbuild/' | grep -v '/debian_build/' | grep -v '/autom4te.cache/' | grep -v '/m4/' | grep -v '/install-sh' | grep -v '/ltmain.sh' | tr '\n' ' '`
+
+shellcheck:
+	@if type shellcheck > /dev/null 2>&1; then \
+		echo "*** Check all files with ShellCheck"; \
+		if [ -n "$(SHELLCHECK_FILES_NO_SH)" ]; then \
+			LC_ALL=C.UTF-8 $(SHELLCHECK_CMD) $(SHELLCHECK_BASE_OPT) $(SHELLCHECK_IGN_OPT) $(SHELLCHECK_FILES_NO_SH) || exit 1; \
+		fi; \
+		if [ -n "$(SHELLCHECK_FILES_SH)" ]; then \
+			LC_ALL=C.UTF-8 $(SHELLCHECK_CMD) $(SHELLCHECK_BASE_OPT) $(SHELLCHECK_IGN_OPT) $(SHELLCHECK_FILES_SH) || exit 1; \
+		fi; \
+		if [ -n "$(SHELLCHECK_FILES_INCLUDE_SH)" ]; then \
+			LC_ALL=C.UTF-8 $(SHELLCHECK_CMD) $(SHELLCHECK_BASE_OPT) $(SHELLCHECK_INCLUDE_IGN_OPT) $(SHELLCHECK_FILES_INCLUDE_SH) || exit 1; \
+		fi; \
+		echo "   -> No error was detected."; \
+		echo ""; \
+	else \
+		echo "ShellCheck is not installed, so skip check by it."; \
+		echo ""; \
+	fi
 
 #
 # Local variables:

diff --git a/buildutils/make_description.sh b/buildutils/make_description.sh
@@ -179,7 +179,7 @@ while IFS= read -r ONE_LINE; do
 				echo " .${ESC_LF_CHAR}"
 			fi
 		else
-			if [ "X${ONE_LINE}" = "X${REVERTED_LINE}" ]; then
+			if [ -n "${ONE_LINE}" ] && [ "${ONE_LINE}" = "${REVERTED_LINE}" ]; then
 				#
 				# This is new section
 				#

diff --git a/buildutils/make_variables.sh b/buildutils/make_variables.sh
@@ -224,7 +224,7 @@ elif [ "${PRGMODE}" = "DEBHELPER" ]; then
 			RESULT="debhelper (>= 9.20160709)${DEB_WITH_SYSTEMD_STRING}, autotools-dev"
 
 		elif [ "${OS_ID_STRING}" = "ubuntu" ]; then
-			if [ ${DEBHELPER_MAJOR_VER} -lt 10 ]; then
+			if [ "${DEBHELPER_MAJOR_VER}" -lt 10 ]; then
 				RESULT="debhelper (>= 9.20160709)${DEB_WITH_SYSTEMD_STRING}, autotools-dev"
 			else
 				RESULT="debhelper (>= 9.20160709)${DEB_WITH_SYSTEMD_STRING}"

diff --git a/lib/k2htpdtorman.cc b/lib/k2htpdtorman.cc
@@ -504,7 +504,6 @@ bool K2HtpDtorManager::LoadConfigurationYaml(const char* config, string& chmpxco
 		// open configuration file
 		if(NULL == (fp = fopen(config, "r"))){
 			ERR_K2HPRN("Could not open configuration file(%s). errno = %d", config, errno);
-			// cppcheck-suppress unmatchedSuppression
 			// cppcheck-suppress resourceLeak
 			return false;
 		}

diff --git a/src/k2htpdtorsvr.cc b/src/k2htpdtorsvr.cc
@@ -131,7 +131,6 @@ static bool ParamParser(int argc, char** argv, dtorparam_t& params)
 				fprintf(stderr, "[ERROR] Option %s needs parameter.\n", argv[cnt]);
 				return false;
 			}
-			// cppcheck-suppress unmatchedSuppression
 			// cppcheck-suppress knownConditionTrueFalse
 			if(is_conf_opt){
 				fprintf(stderr, "[ERROR] Already set -conf option, option %s could not be specified with -conf.\n", argv[cnt]);
@@ -174,7 +173,6 @@ int main(int argc, char** argv)
 	dtorparam_t	params;
 
 	// Parse parameter
-	// cppcheck-suppress unmatchedSuppression
 	// cppcheck-suppress stlSize
 	if(!ParamParser(argc, argv, params) || 0 == params.size()){
 		PrintUsage(argv[0]);

diff --git a/src/k2htpdtorsvrparser.cc b/src/k2htpdtorsvrparser.cc
@@ -763,7 +763,6 @@ static bool ParseK2htpdtorsvrYaml(const char* config, PK2HTPDTORSVRINFO pInfo, b
 		// open configuration file
 		if(NULL == (fp = fopen(config, "r"))){
 			ERR_K2HPRN("Could not open configuration file(%s). errno = %d", config, errno);
-			// cppcheck-suppress unmatchedSuppression
 			// cppcheck-suppress resourceLeak
 			return false;
 		}

diff --git a/src/k2htpdtorsvrplugin.cc b/src/k2htpdtorsvrplugin.cc
@@ -151,15 +151,13 @@ void* K2htpSvrPlugin::WatchChildExit(void* param)
 	}
 
 	// loop
-	// cppcheck-suppress unmatchedSuppression
 	// cppcheck-suppress knownConditionTrueFalse
 	while(!pPlugin->thread_exit){
 		// blocking child exit
 		int		status	= 0;
 		pid_t	childpid= waitpid(-1, &status, 0);
 
 		// check before run plugins
-		// cppcheck-suppress unmatchedSuppression
 		// cppcheck-suppress knownConditionTrueFalse
 		if(pPlugin->thread_exit){
 			break;