Merge pull request #133 from bigbrett/static-analysis-basic-take2

CI static analysis + fixes

Author: billphipps

.github/workflows/static-analysis.yml

@@ -0,0 +1,57 @@
+name: Static Analysis
+
+on:
+  pull_request:
+    branches: [ main, master ]
+  push:
+    branches: [ main, master ]
+
+jobs:
+  cppcheck:
+    runs-on: ubuntu-latest
+
+    steps:
+    - name: Checkout code
+      uses: actions/checkout@v3
+
+    - name: Install cppcheck
+      run: |
+        sudo apt-get update
+        sudo apt-get install -y cppcheck
+
+    - name: Run cppcheck
+      id: cppcheck
+      continue-on-error: true
+      run: |
+        chmod +x tools/static-analysis/run_cppcheck.sh
+        tools/static-analysis/run_cppcheck.sh
+
+    - name: Display errors and warnings
+      if: always()
+      run: |
+        if [ -f tools/static-analysis/reports/cppcheck_summary.txt ]; then
+          ERROR_COUNT=$(grep -c "error:" tools/static-analysis/reports/cppcheck_summary.txt 2>/dev/null) || ERROR_COUNT=0
+          WARNING_COUNT=$(grep -c "warning:" tools/static-analysis/reports/cppcheck_summary.txt 2>/dev/null) || WARNING_COUNT=0
+          STYLE_COUNT=$(grep -c "style:" tools/static-analysis/reports/cppcheck_summary.txt 2>/dev/null) || STYLE_COUNT=0
+
+          echo "## Static Analysis Summary"
+          echo "- Errors: $ERROR_COUNT"
+          echo "- Warnings: $WARNING_COUNT"
+          echo "- Style issues: $STYLE_COUNT (informational only)"
+
+          if [ "$ERROR_COUNT" -gt 0 ] || [ "$WARNING_COUNT" -gt 0 ]; then
+            echo ""
+            echo "### Issues that must be fixed:"
+            echo ""
+            # Show only errors and warnings, not style issues
+            grep -E "(error|warning):" tools/static-analysis/reports/cppcheck_summary.txt || true
+          fi
+        else
+          echo "⚠️ No cppcheck summary file found"
+        fi
+
+    - name: Fail if issues found
+      if: steps.cppcheck.outcome == 'failure'
+      run: |
+        echo "❌ Static analysis failed - errors or warnings were found"
+        exit 1

.gitignore

@@ -1,11 +1,16 @@
-.DS_Store
-Build/
-*.o
-*.a
-*.la
-**/.gdb_history
-tools/testcertgen/ca/
-tools/testcertgen/*.der
-*.code-workspace
-.vscode
-
+.DS_Store
+Build/
+*.o
+*.a
+*.la
+**/.gdb_history
+tools/testcertgen/ca/
+tools/testcertgen/*.der
+*.code-workspace
+.vscode
+
+# Static analysis reports
+tools/static-analysis/reports/
+*.xml
+*.html
+

src/wh_client.c

@@ -97,10 +97,12 @@ int wh_Client_Init(whClientContext* c, const whClientConfig* config)
             }
 
 #ifdef WOLFHSM_CFG_DMA
-            rc = wc_CryptoCb_RegisterDevice(WH_DEV_ID_DMA,
-                                            wh_Client_CryptoCbDma, c);
-            if (rc != 0) {
-                rc = WH_ERROR_ABORTED;
+            if (rc == 0) {
+                rc = wc_CryptoCb_RegisterDevice(WH_DEV_ID_DMA,
+                                                wh_Client_CryptoCbDma, c);
+                if (rc != 0) {
+                    rc = WH_ERROR_ABORTED;
+                }
             }
 #endif /* WOLFHSM_CFG_DMA */
         }
@@ -1498,4 +1500,4 @@ int wh_Client_KeyExportDma(whClientContext* c, uint16_t keyId,
 
 #endif /* WOLFHSM_CFG_DMA */
 
-#endif /* WOLFHSM_CFG_ENABLE_CLIENT */
+#endif /* WOLFHSM_CFG_ENABLE_CLIENT */

src/wh_client_she.c

@@ -120,11 +120,9 @@ int wh_Client_SheSecureBoot(whClientContext* c, uint8_t* bootloader,
 {
     int      ret;
     uint32_t bootloaderSent = 0;
-    uint32_t justSent       = 0;
     uint16_t group;
     uint16_t action;
     uint16_t dataSz;
-    uint8_t* in;
     uint8_t* respBuf;
 
     whMessageShe_SecureBootInitRequest*    initReq    = NULL;
@@ -141,9 +139,6 @@ int wh_Client_SheSecureBoot(whClientContext* c, uint8_t* bootloader,
         (whMessageShe_SecureBootInitRequest*)wh_CommClient_GetDataPtr(c->comm);
     respBuf = (uint8_t*)wh_CommClient_GetDataPtr(c->comm);
 
-    /* in is after the size argument */
-    in = (uint8_t*)(initReq + 1);
-
     /* send init sub command */
     initReq->sz = bootloaderLen;
     ret =
@@ -159,6 +154,9 @@ int wh_Client_SheSecureBoot(whClientContext* c, uint8_t* bootloader,
 
     /* send update sub command until we've sent the entire bootloader */
     while (ret == 0 && bootloaderSent < bootloaderLen) {
+        uint8_t* in;
+        uint32_t justSent;
+
         if (initResp->rc != WH_SHE_ERC_NO_ERROR) {
             return initResp->rc;
         }

src/wh_nvm_flash.c

@@ -442,6 +442,12 @@ static int nfPartition_ProgramCount(whNvmFlashContext* context,
 
 static int nfPartition_ProgramInit(whNvmFlashContext* context, int partition)
 {
+    int ret = 0;
+
+    if ((context == NULL) || (context->cb == NULL)) {
+        return WH_ERROR_BADARGS;
+    }
+
     /* Valid initial state values for a partition */
     nfMemState init_state =
     {
@@ -450,11 +456,6 @@ static int nfPartition_ProgramInit(whNvmFlashContext* context, int partition)
         .start = NF_PARTITION_DATA_OFFSET,
         .count = context->partition_units,
     };
-    int ret = 0;
-
-    if ((context == NULL) || (context->cb == NULL)) {
-        return WH_ERROR_BADARGS;
-    }
 
     /* Blankcheck/Erase partition */
     ret = nfPartition_BlankCheck(context, partition);
@@ -735,12 +736,11 @@ static int nfObject_Copy(whNvmFlashContext* context, int object_index,
     }
     ret = nfObject_ProgramFinish(context, partition, dest_object, data_len);
     if (ret != 0) return ret;
+
     dest_object++;
+    *inout_next_object = dest_object;
+    *inout_next_data = dest_data;
 
-    if (ret == 0) {
-        *inout_next_object = dest_object;
-        *inout_next_data = dest_data;
-    }
     return ret;
 }