Add cppcheck (#7)

.github/workflows/cppcheck.yml

@@ -0,0 +1,32 @@
+name: cppcheck
+on: [push]
+permissions:
+  contents: read
+defaults:
+  run:
+    shell: bash
+jobs:
+  build:
+    runs-on: ubuntu-24.04
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4
+        with:
+          submodules: recursive
+      - name: Setup Environment
+        run: |
+          sudo apt-get install tcl8.6-dev tcl-vfs cppcheck
+          echo "CFGOPT_COMMON=--with-tcl=/usr/lib/tcl8.6" >> $GITHUB_ENV
+      - name: Configure
+        run: |
+          ./configure $CFGOPT_COMMON || {
+            cat config.log
+            echo "::error::Failure during Configure"
+            exit 1
+          }
+      - name: Run cppcheck
+        run: |
+          make cppcheck || {
+            echo "::error::Failure during cppcheck"
+            exit 1
+          }

ChangeLog

@@ -1,3 +1,6 @@
+2024-05-28 Konstantin Kushnir <[email protected]>
+	* Add cppcheck
+
 2024-05-27 Konstantin Kushnir <[email protected]>
 	* Fix a bug when deleting the last entry from small file buffer
 	* Add support for optimize list command for c-vfs

Makefile.in

@@ -330,6 +330,15 @@ $(srcdir)/doc/%.n: $(srcdir)/doc/%.man
 	cd "$(dir $@)" && echo 'package require dtplite; dtplite::do [list -o $(notdir $@) nroff $(notdir $<)]' | \
 	    $(TCLSH)
 
+.PHONY: cppcheck
+cppcheck:
+	cppcheck -q -j$(shell nproc) --enable=all --inconclusive --inline-suppr --force --error-exitcode=1 \
+	    $(INCLUDES) \
+	    --suppress=missingIncludeSystem \
+	    --suppress=constParameterPointer \
+	    --suppress=constVariablePointer \
+	    "$(srcdir)/generic"
+
 ifeq ($(CFLAGS_DEFAULT),-g)
     PACKAGE_ARCHIVE = $(PACKAGE_NAME)-debug-$(PACKAGE_VERSION)
 else

generic/bindata.c

@@ -91,7 +91,6 @@ static int CookfsBinaryDataParseAddress(Tcl_Interp *interp, Tcl_Obj *address, vo
     }
 }
 
-
 static Tcl_Obj *CookfsBinaryDataCreateAddress(void *address) {
     char buf[128];
     sprintf(buf, "%p", address);

generic/common.c

@@ -29,15 +29,14 @@
  */
 
 unsigned char *Cookfs_Binary2Int(unsigned char *input, int *output, int count) {
-    int b;
     while (count > 0) {
         /*
             Avoid undefined behaviour warning:
                 runtime error: left shift of 255 by 24 places cannot be represented in type 'int'
             because even if input[0] is an unsigned char, it is still promoted
             to a normal signed int.
         */
-        b = ((unsigned int)input[0] << 24)
+        int b = ((unsigned int)input[0] << 24)
             | (input[1] << 16)
             | (input[2] << 8)
             | input[3];
@@ -103,9 +102,8 @@ unsigned char *Cookfs_Int2Binary(int *input, unsigned char *output, int count) {
  */
 
 unsigned char *Cookfs_Binary2WideInt(unsigned char *input, Tcl_WideInt *output, int count) {
-    Tcl_WideInt v;
     while (count > 0) {
-        v = input[0]; v = v << 8;
+        Tcl_WideInt v = input[0]; v = v << 8;
         v |= input[1]; v = v << 8;
         v |= input[2]; v = v << 8;
         v |= input[3]; v = v << 8;
@@ -142,9 +140,8 @@ unsigned char *Cookfs_Binary2WideInt(unsigned char *input, Tcl_WideInt *output,
  */
 
 unsigned char *Cookfs_WideInt2Binary(Tcl_WideInt *input, unsigned char *output, int count) {
-    Tcl_WideInt v;
     while (count > 0) {
-        v = *input;
+        Tcl_WideInt v = *input;
         output[7] = v & ((Tcl_WideInt) 0x000000ff); v = v >> 8;
         output[6] = v & ((Tcl_WideInt) 0x000000ff); v = v >> 8;
         output[5] = v & ((Tcl_WideInt) 0x000000ff); v = v >> 8;

generic/cookfs.c

@@ -26,7 +26,7 @@
  */
 
 DLLEXPORT int
-Cookfs_Init(Tcl_Interp *interp)
+Cookfs_Init(Tcl_Interp *interp) // cppcheck-suppress unusedFunction
 {
     char buf[256];
     if (Tcl_InitStubs(interp, "8.5", 0) == NULL) {

generic/fsindex.c

@@ -15,7 +15,6 @@
 #define COOKFSFSINDEX_FIND_DELETE_RECURSIVE 3
 
 /* declarations of static and/or internal functions */
-Cookfs_FsindexEntry *CookfsFsindexFindElement(Cookfs_Fsindex *i, Tcl_Obj *pathList, int listSize);
 static Cookfs_FsindexEntry *CookfsFsindexFind(Cookfs_Fsindex *i, Cookfs_FsindexEntry **dirPtr, Tcl_Obj *pathList, int command, Cookfs_FsindexEntry *newFileNode);
 static Cookfs_FsindexEntry *CookfsFsindexFindInDirectory(Cookfs_FsindexEntry *currentNode, char *pathTailStr, int command, Cookfs_FsindexEntry *newFileNode);
 static void CookfsFsindexChildtableToHash(Cookfs_FsindexEntry *e);
@@ -477,7 +476,7 @@ Cookfs_FsindexEntry *Cookfs_FsindexGet(Cookfs_Fsindex *i, Tcl_Obj *pathList) {
 Cookfs_FsindexEntry *Cookfs_FsindexSet(Cookfs_Fsindex *i, Tcl_Obj *pathList, int numBlocks) {
     Cookfs_FsindexEntry *dirNode = NULL;
     Cookfs_FsindexEntry *fileNode;
-    Cookfs_FsindexEntry *foundFileNode;
+    const Cookfs_FsindexEntry *foundFileNode;
     Tcl_Obj *pathTail = NULL;
     char *pathTailStr;
     int pathTailLen;
@@ -553,7 +552,7 @@ Cookfs_FsindexEntry *Cookfs_FsindexSet(Cookfs_Fsindex *i, Tcl_Obj *pathList, int
 
 Cookfs_FsindexEntry *Cookfs_FsindexSetInDirectory(Cookfs_FsindexEntry *currentNode, char *pathTailStr, int pathTailLen, int numBlocks) {
     Cookfs_FsindexEntry *fileNode;
-    Cookfs_FsindexEntry *foundFileNode;
+    const Cookfs_FsindexEntry *foundFileNode;
     CookfsLog(printf("Cookfs_FsindexSetInDirectory - begin (%s/%d)", pathTailStr, pathTailLen))
     fileNode = Cookfs_FsindexEntryAlloc(pathTailLen, numBlocks, COOKFS_USEHASH_DEFAULT);
     strcpy(fileNode->fileName, pathTailStr);
@@ -589,7 +588,7 @@ Cookfs_FsindexEntry *Cookfs_FsindexSetInDirectory(Cookfs_FsindexEntry *currentNo
  */
 
 int Cookfs_FsindexUnset(Cookfs_Fsindex *i, Tcl_Obj *pathList) {
-    Cookfs_FsindexEntry *fileNode;
+    const Cookfs_FsindexEntry *fileNode;
 
     CookfsLog(printf("Cookfs_FsindexUnset - start"))
 
@@ -622,7 +621,7 @@ int Cookfs_FsindexUnset(Cookfs_Fsindex *i, Tcl_Obj *pathList) {
  */
 
 int Cookfs_FsindexUnsetRecursive(Cookfs_Fsindex *i, Tcl_Obj *pathList) {
-    Cookfs_FsindexEntry *fileNode;
+    const Cookfs_FsindexEntry *fileNode;
 
     CookfsLog(printf("Cookfs_FsindexUnsetRecursive - start"))
 
@@ -663,8 +662,6 @@ Cookfs_FsindexEntry **Cookfs_FsindexListEntry(Cookfs_FsindexEntry *dirNode, int
     Cookfs_FsindexEntry *itemNode;
     Cookfs_FsindexEntry **result;
     int idx = 0;
-    Tcl_HashSearch hashSearch;
-    Tcl_HashEntry *hashEntry;
 
     CookfsLog(printf("Cookfs_FsindexListEntry - start"))
 
@@ -682,7 +679,8 @@ Cookfs_FsindexEntry **Cookfs_FsindexListEntry(Cookfs_FsindexEntry *dirNode, int
 
     CookfsLog(printf("Cookfs_FsindexListEntry - isHash=%d", dirNode->data.dirInfo.isHash))
     if (dirNode->data.dirInfo.isHash) {
-	hashEntry = Tcl_FirstHashEntry(&dirNode->data.dirInfo.dirData.children, &hashSearch);
+        Tcl_HashSearch hashSearch;
+	Tcl_HashEntry *hashEntry = Tcl_FirstHashEntry(&dirNode->data.dirInfo.dirData.children, &hashSearch);
 	while (hashEntry != NULL) {
 	    itemNode = (Cookfs_FsindexEntry *) Tcl_GetHashValue(hashEntry);
 	    result[idx] = itemNode;
@@ -851,15 +849,13 @@ void Cookfs_FsindexEntryFree(Cookfs_FsindexEntry *e) {
     //CookfsLog(printf("Cookfs_FsindexEntryFree: %p with fileBlocks [%d]", e, e->fileBlocks));
     if (e->fileBlocks == COOKFS_NUMBLOCKS_DIRECTORY) {
 	/* for directory, recursively free all children */
-        Cookfs_FsindexEntry *itemNode;
-
 	if (e->data.dirInfo.isHash) {
 	    Tcl_HashSearch hashSearch;
 	    Tcl_HashEntry *hashEntry;
 	    /* iterate over hash table for all children */
 	    hashEntry = Tcl_FirstHashEntry(&e->data.dirInfo.dirData.children, &hashSearch);
 	    while (hashEntry != NULL) {
-		itemNode = (Cookfs_FsindexEntry *) Tcl_GetHashValue(hashEntry);
+		Cookfs_FsindexEntry *itemNode = (Cookfs_FsindexEntry *) Tcl_GetHashValue(hashEntry);
 		hashEntry = Tcl_NextHashEntry(&hashSearch);
 		Cookfs_FsindexEntryFree(itemNode);
 	    }
@@ -998,13 +994,12 @@ int Cookfs_FsindexUnsetMetadata(Cookfs_Fsindex *i, const char *paramName) {
  *
  *----------------------------------------------------------------------
  */
-Cookfs_FsindexEntry *CookfsFsindexFindElement(Cookfs_Fsindex *i, Tcl_Obj *pathList, int listSize) {
+Cookfs_FsindexEntry *CookfsFsindexFindElement(const Cookfs_Fsindex *i, Tcl_Obj *pathList, int listSize) {
     int idx;
     Tcl_Obj *currentPath;
     Tcl_HashEntry *hashEntry;
     Cookfs_FsindexEntry *currentNode;
     Cookfs_FsindexEntry *nextNode;
-    char *currentPathStr;
 
     /* start off with root item */
     currentNode = i->rootItem;
@@ -1028,7 +1023,7 @@ Cookfs_FsindexEntry *CookfsFsindexFindElement(Cookfs_Fsindex *i, Tcl_Obj *pathLi
             return NULL;
         }
         CookfsLog(printf("Got element"))
-        currentPathStr = Tcl_GetStringFromObj(currentPath, NULL);
+        char *currentPathStr = Tcl_GetStringFromObj(currentPath, NULL);
         CookfsLog(printf("Looking for %s", currentPathStr))
 
 
@@ -1042,12 +1037,11 @@ Cookfs_FsindexEntry *CookfsFsindexFindElement(Cookfs_Fsindex *i, Tcl_Obj *pathLi
 	    currentNode = (Cookfs_FsindexEntry *) Tcl_GetHashValue(hashEntry);
 	}  else  {
 	    /* if it is an array of children, iterate and find matching child */
-	    int i;
 	    nextNode = NULL;
 	    CookfsLog(printf("Iterating over childTable to find %s", currentPathStr))
-	    for (i = 0 ; i < COOKFS_FSINDEX_TABLE_MAXENTRIES; i++) {
-		if ((currentNode->data.dirInfo.dirData.childTable[i] != NULL) && (strcmp(currentNode->data.dirInfo.dirData.childTable[i]->fileName, currentPathStr) == 0)) {
-		    nextNode = currentNode->data.dirInfo.dirData.childTable[i];
+	    for (int j = 0 ; j < COOKFS_FSINDEX_TABLE_MAXENTRIES; j++) {
+		if ((currentNode->data.dirInfo.dirData.childTable[j] != NULL) && (strcmp(currentNode->data.dirInfo.dirData.childTable[j]->fileName, currentPathStr) == 0)) {
+		    nextNode = currentNode->data.dirInfo.dirData.childTable[j];
 		    CookfsLog(printf("Iterating over childTable to find %s - FOUND", currentPathStr))
 		    break;
 		}
@@ -1208,6 +1202,7 @@ static Cookfs_FsindexEntry *CookfsFsindexFindInDirectory(Cookfs_FsindexEntry *cu
 		hashEntry = Tcl_CreateHashEntry(&currentNode->data.dirInfo.dirData.children, pathTailStr, &isNew);
 	    }  else  {
 		hashEntry = Tcl_FindHashEntry(&currentNode->data.dirInfo.dirData.children, pathTailStr);
+		isNew = 0;
 	    }
 
 	    /* if entry has been found/created */

generic/fsindex.h

@@ -77,7 +77,7 @@ int Cookfs_FsindexUnsetMetadata(Cookfs_Fsindex *i, const char *paramName);
 int Cookfs_FsindexGetBlockUsage(Cookfs_Fsindex *i, int idx);
 void Cookfs_FsindexModifyBlockUsage(Cookfs_Fsindex *i, int idx, int count);
 
-Cookfs_FsindexEntry *CookfsFsindexFindElement(Cookfs_Fsindex *i, Tcl_Obj *pathList, int listSize);
+Cookfs_FsindexEntry *CookfsFsindexFindElement(const Cookfs_Fsindex *i, Tcl_Obj *pathList, int listSize);
 
 Tcl_WideInt Cookfs_FsindexIncrChangeCount(Cookfs_Fsindex *i, int count);
 void Cookfs_FsindexResetChangeCount(Cookfs_Fsindex *i);

generic/fsindexCmd.c

@@ -545,7 +545,6 @@ static int CookfsFsindexCmdSetmtime(Cookfs_Fsindex *fsIndex, Tcl_Interp *interp,
  */
 
 static int CookfsFsindexCmdSet(Cookfs_Fsindex *fsIndex, Tcl_Interp *interp, int objc, Tcl_Obj *const objv[]) {
-    int i;
     int numBlocks;
     int fileBlockData;
     Tcl_Obj *splitPath;
@@ -598,6 +597,7 @@ static int CookfsFsindexCmdSet(Cookfs_Fsindex *fsIndex, Tcl_Interp *interp, int
 	}
 	entry->data.fileInfo.fileSize = 0;
 
+	int i;
 	/* copy all integers from filedata into newly created entry */
 	for (i = 0; i < numBlocks * 3; i++) {
 	    if (Tcl_GetIntFromObj(interp, listElements[i], &fileBlockData) != TCL_OK) {
@@ -704,7 +704,6 @@ static int CookfsFsindexCmdUnset(Cookfs_Fsindex *fsIndex, Tcl_Interp *interp, in
 static int CookfsFsindexCmdGet(Cookfs_Fsindex *fsIndex, Tcl_Interp *interp, int objc, Tcl_Obj *const objv[]) {
     Tcl_Obj *splitPath;
     Tcl_Obj *resultObjects[3];
-    Tcl_Obj **resultList;
     Cookfs_FsindexEntry *entry;
 
     /* check arguments */
@@ -739,6 +738,7 @@ static int CookfsFsindexCmdGet(Cookfs_Fsindex *fsIndex, Tcl_Interp *interp, int
 	Tcl_SetObjResult(interp, Tcl_NewListObj(1, resultObjects));
     }  else  {
 	int i;
+	Tcl_Obj **resultList;
 
 	/* for files, store file size and create a sublist with block-offset-size triplets */
 	resultObjects[1] = Tcl_NewWideIntObj(entry->data.fileInfo.fileSize);
@@ -806,6 +806,12 @@ static int CookfsFsindexCmdList(Cookfs_Fsindex *fsIndex, Tcl_Interp *interp, int
     /* create a file list from result of Cookfs_FsindexList() */
     resultList = (Tcl_Obj **) ckalloc(sizeof(Tcl_Obj *) * itemCount);
     for (idx = 0; idx < itemCount; idx++) {
+        // Due to unknown reason, cppcheck gives here the following:
+        //     Argument 'results[idx]->fileName,-1' to function tcl_NewStringObj
+        //     is always -1. It does not matter what value 'idx'
+        //     has. [knownArgument]
+        // This sounds like a bug in cppcheck.
+        // cppcheck-suppress knownArgument
 	resultList[idx] = Tcl_NewStringObj(results[idx]->fileName, -1);
     }