Add PR workflow for Static Code Analyzer and fix existing errors and …

…warnings (#2143) This commit integrates Static Code Analyzer with PR workflow to improve code quality and catch errors quickly, along with that this commit also fixes the existing errors and warnings. Signed-off-by: Sumit Jaiswal [email protected]
babelfish-for-postgresql · Dec 19, 2023 · dff6a8f · dff6a8f
1 parent f1015e6
commit dff6a8f
Show file tree

Hide file tree

Showing 31 changed files with 180 additions and 79 deletions.
diff --git a/.github/workflows/static-code-analyzer.yml b/.github/workflows/static-code-analyzer.yml
@@ -0,0 +1,53 @@
+name: Static Code Analyzer
+on: [push, pull_request]
+
+jobs:
+  run-static-code-analyzer:
+    runs-on: ubuntu-22.04
+    steps:
+      - uses: actions/checkout@v2
+        id: checkout
+
+      - name: Install cppcheck
+        id: install-cppcheck
+        if: always()
+        run: |
+          sudo apt-get install cppcheck
+
+      - name: Clone engine repository
+        id: clone-engine-repo
+        if: always() && steps.install-cppcheck.outcome == 'success'
+        run: |
+          cd ..
+          rm -rf postgresql_modified_for_babelfish
+
+          if [[ $GITHUB_EVENT_NAME == "pull_request" ]]; then
+            ENGINE_BRANCH=$GITHUB_HEAD_REF
+          else
+            ENGINE_BRANCH=$GITHUB_REF_NAME
+          fi
+
+          $GITHUB_WORKSPACE/.github/scripts/clone_engine_repo "$GITHUB_REPOSITORY_OWNER" "$ENGINE_BRANCH"
+
+      - name: Run cppcheck on extensions
+        id: run-cppcheck
+        if: always()
+        run: |
+          export NPROC=`nproc`
+
+          cppcheck --error-exitcode=-1 -j $NPROC --template=gcc \
+          --enable=warning --inline-suppr \
+          ./contrib \
+          --suppress=nullPointerRedundantCheck \
+          --suppress=unknownMacro \
+          --output-file=cppcheck-failures.txt \
+          --include=../postgresql_modified_for_babelfish/src/include/utils/elog.h \
+          --include=../postgresql_modified_for_babelfish/src/include/c.h 
+      
+      - name: Upload failures as artifacts
+        id: upload-failures
+        if: always() && steps.run-cppcheck.outcome == 'failure'
+        uses: actions/upload-artifact@v2
+        with:
+          name: cppcheck-failures
+          path: ./cppcheck-failures.txt
diff --git a/contrib/babelfishpg_common/src/collation.c b/contrib/babelfishpg_common/src/collation.c
@@ -735,7 +735,7 @@ init_collid_trans_tab_internal(void)
 	Oid			nspoid;
 	ht_oid2collid_entry *entry;
 	int			locale_pos = -1;
-	char	   *atsign;
+	char	   *atsign = NULL;
 	char	   *locale;
 
 	if (TransMemoryContext == NULL) /* initialize memory context */
@@ -788,18 +788,22 @@ init_collid_trans_tab_internal(void)
 			init_default_locale();
 
 			locale = pstrdup(bbf_default_locale);
-			atsign = strstr(locale, "@");
+			if (locale)
+				atsign = strstr(locale, "@");
 			if (atsign != NULL)
 				*atsign = '\0';
 			locale_pos = find_locale(locale);
 
 			if (locale_pos < 0)
+			{
 				ereport(ERROR,
 						(errcode(ERRCODE_INTERNAL_ERROR),
 						 errmsg("invalid setting detected for babelfishpg_tsql.default_locale setting")));
+			}
 			coll_infos[i].lcid = locales[locale_pos].lcid;
 			coll_infos[i].code_page = locales[locale_pos].code_page;
 			coll_infos[i].enc = locales[locale_pos].enc;
+
 			if (locale)
 				pfree(locale);
 		}
@@ -916,9 +920,19 @@ lookup_like_ilike_table(Oid opno)
 	/* return invalid oid when not found */
 	if (!found)
 	{
-		like_ilike_info invalid;
+		like_ilike_info invalid = {
+			InvalidOid,		/* like_oid */
+			NULL,			/* like_op_name */
+			NULL,			/* ilike_op_name */
+			NULL,			/* op_left_schema */
+			NULL,			/* the name of left operand */
+			NULL,			/* op_left_name */
+			NULL,			/* op_right_name */
+			false,			/* is_not_match */
+			InvalidOid,		/* ilike_oid */
+			InvalidOid		/* ilike_opfuncid */
+		};
 
-		invalid.like_oid = InvalidOid;
 		return invalid;
 	}
 
@@ -960,9 +974,17 @@ lookup_collation_table(Oid coll_oid)
 	{
 		int			collidx;
 
-		coll_info	invalid;
-
-		invalid.oid = InvalidOid;
+		coll_info	invalid = {
+			InvalidOid,		/* oid */
+			NULL,			/* collname */
+			-1,			/* lcid */
+			-1,			/* ver */
+			-1,			/* style */
+			-1,			/* sortid */
+			-1,			/* collateflags */
+			-1,			/* code_page */
+			PG_SQL_ASCII		/* enc */
+		};
 
 		collidx = get_server_collation_collidx();
 		if (collidx == NOT_FOUND)
@@ -1484,10 +1506,9 @@ BabelfishPreCreateCollation_hook(
 Oid
 get_oid_from_collidx(int collidx)
 {
-	if (collidx > NOT_FOUND)
-		return coll_infos[collidx].oid;
-
-	return InvalidOid;
+	if (collidx == NOT_FOUND)
+		return InvalidOid;
+	return coll_infos[collidx].oid;
 }
 
 collation_callbacks *

diff --git a/contrib/babelfishpg_common/src/datetime.c b/contrib/babelfishpg_common/src/datetime.c
@@ -73,7 +73,7 @@ datetime_in_str(char *str)
 	struct pg_tm tt,
 			   *tm = &tt;
 	int			tz;
-	int			dtype;
+	int			dtype = -1;
 	int			nf;
 	int			dterr;
 	char	   *field[MAXDATEFIELDS];

diff --git a/contrib/babelfishpg_common/src/datetime2.c b/contrib/babelfishpg_common/src/datetime2.c
@@ -49,7 +49,7 @@ datetime2_in_str(char *str, int32 typmod)
 	struct pg_tm tt,
 			   *tm = &tt;
 	int			tz;
-	int			dtype;
+	int			dtype = -1;
 	int			nf;
 	int			dterr;
 	char	   *field[MAXDATEFIELDS];

diff --git a/contrib/babelfishpg_common/src/datetimeoffset.c b/contrib/babelfishpg_common/src/datetimeoffset.c
@@ -89,7 +89,7 @@ datetimeoffset_in(PG_FUNCTION_ARGS)
 	struct pg_tm tt,
 			   *tm = &tt;
 	int			tz;
-	int			dtype;
+	int			dtype = -1;
 	int			nf;
 	int			dterr;
 	char	   *field[MAXDATEFIELDS];

diff --git a/contrib/babelfishpg_common/src/smalldatetime.c b/contrib/babelfishpg_common/src/smalldatetime.c
@@ -70,7 +70,7 @@ smalldatetime_in_str(char *str)
 	struct pg_tm tt,
 			   *tm = &tt;
 	int			tz;
-	int			dtype;
+	int			dtype = -1;
 	int			nf;
 	int			dterr;
 	char	   *field[MAXDATEFIELDS];

diff --git a/contrib/babelfishpg_common/src/varbinary.c b/contrib/babelfishpg_common/src/varbinary.c
@@ -275,7 +275,10 @@ varbinaryout(PG_FUNCTION_ARGS)
 			 bytea_output);
 		rp = result = NULL;		/* keep compiler quiet */
 	}
-	*rp = '\0';
+
+	if (rp)
+		*rp = '\0';
+
 	PG_RETURN_CSTRING(result);
 }
 

diff --git a/contrib/babelfishpg_tds/src/backend/fault_injection/fault_injection.c b/contrib/babelfishpg_tds/src/backend/fault_injection/fault_injection.c
@@ -88,9 +88,9 @@ FaultInjectionInitialize()
 		/* should not try to insert same entry multiple times */
 		Assert(foundPtr == false);
 
-		if (entry == NULL)
+		if (new_entry == NULL)
 		{
-			ereport(DEBUG5,
+			ereport(ERROR,
 					(errmsg("FaultInjectionLookupHashEntry() could not insert fault injection hash entry:'%s' ",
 							entry->faultName)));
 		}

diff --git a/contrib/babelfishpg_tds/src/backend/fault_injection/fault_injection_tests.c b/contrib/babelfishpg_tds/src/backend/fault_injection/fault_injection_tests.c
@@ -243,6 +243,7 @@ throw_error_buffer(void *arg, int *num_occurrences)
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Warray-bounds"
 #pragma GCC diagnostic ignored "-Wstringop-overflow"
+	/* cppcheck-suppress bufferAccessOutOfBounds */
 	memcpy(buffer, tem, 10);
 #pragma GCC diagnostic pop
 	if (can != 0)

diff --git a/contrib/babelfishpg_tds/src/backend/tds/tdsbulkload.c b/contrib/babelfishpg_tds/src/backend/tds/tdsbulkload.c
@@ -928,7 +928,7 @@ ProcessBCPRequest(TDSRequest request)
 		}
 		PG_CATCH();
 		{
-			int			ret;
+			int			ret = 0;
 
 			HOLD_CANCEL_INTERRUPTS();
 
@@ -989,7 +989,7 @@ ProcessBCPRequest(TDSRequest request)
 			}
 			PG_CATCH();
 			{
-				int			ret;
+				int			ret = 0;
 
 				HOLD_CANCEL_INTERRUPTS();
 				HOLD_INTERRUPTS();

diff --git a/contrib/babelfishpg_tds/src/backend/tds/tdstypeio.c b/contrib/babelfishpg_tds/src/backend/tds/tdstypeio.c
@@ -2395,7 +2395,7 @@ TdsRecvTypeTable(const char *message, const ParameterToken token)
 		{
 			TdsIoFunctionInfo tempFuncInfo;
 			int			currentColumn = 0;
-			char	   *currentQuery = " ";
+			char	   *currentQuery = pstrdup(" ");
 
 			while (currentColumn != token->tvpInfo->colCount)
 			{

diff --git a/contrib/babelfishpg_tsql/runtime/functions.c b/contrib/babelfishpg_tsql/runtime/functions.c
@@ -937,7 +937,7 @@ host_os(PG_FUNCTION_ARGS)
 
 	/* filter out host info */
 	pg_version = pstrdup(PG_VERSION_STR);
-	sscanf(pg_version, "PostgreSQL %*s on %s, compiled by %*s", host_str);
+	sscanf(pg_version, "PostgreSQL %*255s on %255s, compiled by %*255s", host_str);
 
 	if (strstr(host_str, "w64") || strstr(host_str, "w32") || strstr(host_str, "mingw") || strstr(host_str, "visual studio"))
 	{
@@ -3748,7 +3748,7 @@ int
 SPI_execute_raw_parsetree(RawStmt *parsetree, const char * sourcetext, bool read_only, long tcount)
 {
 	_SPI_plan			plan;
-	int					ret;
+	int					ret = 0;
 	List				*plancache_list;
 	CachedPlanSource	*plansource;
 	int					prev_sql_dialect;

diff --git a/contrib/babelfishpg_tsql/src/backend_parser/gram-tsql-epilogue.y.c b/contrib/babelfishpg_tsql/src/backend_parser/gram-tsql-epilogue.y.c
@@ -414,7 +414,7 @@ tsql_check_param_readonly(const char *paramname, TypeName *typename, bool readon
 	TypeName   *typeclone = copyObjectImpl(typename);
 
 	/* work on the cloned object to avoid double rewriting */
-	rewrite_plain_name(typeclone->names);
+	typeclone->names = rewrite_plain_name(typeclone->names);
 	if (typeidTypeRelid(typenameTypeId(NULL, typeclone)) == InvalidOid)
 	{
 		/* Not table-valued parameter - must not be READONLY */

diff --git a/contrib/babelfishpg_tsql/src/cursor.c b/contrib/babelfishpg_tsql/src/cursor.c
@@ -1044,7 +1044,7 @@ execute_sp_cursor(int cursor_handle, int opttype, int rownum, const char *tablen
 
 	validate_sp_cursor_params(opttype, rownum, tablename, values);
 
-	snprintf(curname, NAMEDATALEN, "%u", cursor_handle);
+	snprintf(curname, NAMEDATALEN, "%d", cursor_handle);
 	hentry = (CursorHashEnt *) hash_search(CursorHashTable, curname, HASH_FIND, NULL);
 	if (hentry == NULL)
 		elog(ERROR, "cursor \"%s\" does not exist", curname);
@@ -1171,7 +1171,7 @@ execute_sp_cursorfetch(int cursor_handle, int *pfetchtype, int *prownum, int *pn
 	PortalContext = savedPortalCxt;
 
 	/* find cursor entry, validate input options and open portal */
-	snprintf(curname, NAMEDATALEN, "%u", cursor_handle);
+	snprintf(curname, NAMEDATALEN, "%d", cursor_handle);
 	hentry = (CursorHashEnt *) hash_search(CursorHashTable, curname, HASH_FIND, NULL);
 	if (hentry == NULL)
 		elog(ERROR, "cursor \"%s\" does not exist", curname);
@@ -1307,7 +1307,7 @@ execute_sp_cursoroption(int cursor_handle, int code, int value)
 
 	validate_sp_cursoroption_params(code, value);
 
-	snprintf(curname, NAMEDATALEN, "%u", cursor_handle);
+	snprintf(curname, NAMEDATALEN, "%d", cursor_handle);
 	hentry = (CursorHashEnt *) hash_search(CursorHashTable, curname, HASH_FIND, NULL);
 	if (hentry == NULL)
 		elog(ERROR, "cursor \"%s\" does not exist", curname);
@@ -1403,7 +1403,7 @@ execute_sp_cursorclose(int cursor_handle)
 		elog(ERROR, "SPI_connect failed: %s", SPI_result_code_string(rc));
 	PortalContext = savedPortalCxt;
 
-	snprintf(curname, NAMEDATALEN, "%u", cursor_handle);
+	snprintf(curname, NAMEDATALEN, "%d", cursor_handle);
 	hentry = (CursorHashEnt *) hash_search(CursorHashTable, curname, HASH_FIND, NULL);
 	if (hentry == NULL)
 		elog(ERROR, "cursor \"%s\" does not exist", curname);
@@ -1515,7 +1515,8 @@ execute_sp_cursoropen_common(int *stmt_handle, int *cursor_handle, const char *s
 
 		*cursor_handle = get_next_cursor_handle();
 
-		snprintf(curname, NAMEDATALEN, "%u", *cursor_handle);
+		if (cursor_handle != NULL)
+			snprintf(curname, NAMEDATALEN, "%d", *cursor_handle);
 
 		/* add new cursor entry */
 		hentry = pltsql_insert_cursor_entry(curname, NULL, cursor_options, cursor_handle);

diff --git a/contrib/babelfishpg_tsql/src/extendedproperty.c b/contrib/babelfishpg_tsql/src/extendedproperty.c
@@ -254,10 +254,10 @@ sp_dropextendedproperty(PG_FUNCTION_ARGS)
 static void
 sp_execextended_property(PG_FUNCTION_ARGS, ExtendedPropertyProc proc)
 {
-	char		*name, *orig_name,
-				*level0type, *level0name,
-				*level1type, *level1name,
-				*level2type, *level2name;
+	char		*name = NULL, *orig_name = NULL,
+				*level0type = NULL, *level0name = NULL,
+				*level1type = NULL, *level1name = NULL,
+				*level2type = NULL, *level2name = NULL;
 	bytea		*value = NULL;
 	int16		db_id;
 	const char	*type;

diff --git a/contrib/babelfishpg_tsql/src/hooks.c b/contrib/babelfishpg_tsql/src/hooks.c
@@ -1739,6 +1739,7 @@ pre_transform_target_entry(ResTarget *res, ParseState *pstate,
 			if(actual_alias_len > alias_len)
 			{
 				/* First 32 characters of original_name are assigned to alias. */
+				/* cppcheck-suppress invalidFunctionArg */
 				memcpy(alias, original_name, (alias_len - 32));
 
 				/* Last 32 characters of identifier_name are assigned to alias, as actual alias is truncated. */

diff --git a/contrib/babelfishpg_tsql/src/iterative_exec.c b/contrib/babelfishpg_tsql/src/iterative_exec.c
@@ -595,7 +595,7 @@ get_stat_trace(ExecCodes *exec_code, ExecStat *stat, StringInfo buf)
 
 	/* Header */
 	snprintf(local_buf, TRACE_LOCAL_BUF_SIZE,
-			 "Execution Summary: %s.%s total execution code size %zu, total execution time %zums\n",
+			 "Execution Summary: %s.%s total execution code size %zu, total execution time %lums\n",
 			 exec_code->proc_namespace, exec_code->proc_name,
 			 code_size, stat->total_duration);
 	appendStringInfoString(buf, local_buf);
@@ -1191,7 +1191,7 @@ dispatch_stmt_handle_error(PLtsql_execstate *estate,
 	int			rc = PLTSQL_RC_OK;
 	volatile bool internal_sp_started;
 	volatile int before_lxid = MyProc->lxid;
-	volatile int before_subtxn_id;
+	volatile int before_subtxn_id = 0;
 	MemoryContext cur_ctxt = CurrentMemoryContext;
 	ResourceOwner oldowner = CurrentResourceOwner;
 	SimpleEcontextStackEntry *volatile topEntry = simple_econtext_stack;

diff --git a/contrib/babelfishpg_tsql/src/linked_servers.c b/contrib/babelfishpg_tsql/src/linked_servers.c
@@ -679,7 +679,7 @@ tdsTypeTypmod(int datatype, int datalen, bool is_metadata, int precision, int sc
 			{
 				LINKED_SERVER_DEBUG_FINER("LINKED SERVER: time info - scale: %d", scale);
 
-				if (scale >= 0 || scale < 7)
+				if (scale >= 0 && scale < 7)
 					return scale;
 				else
 					return -1;