RDKEMW-22276: Coverity integration for networkmanager plugin

[gururaajar]

Coverity integration for networkmanager plugin

[Copilot]

Pull request overview

Adds a standalone cov_build.sh script to reproduce the libnm_proxy_l1_tests GitHub Actions build locally in a Coverity-friendly way (build-only: no test/coverage/upload), by building ThunderTools/Thunder/ThunderInterfaces and then configuring/building NetworkManager with the GNOME libnm proxy settings.

Changes:

• Introduces a local build script that mirrors the CI workflow’s dependency installation, Thunder repo setup, and build steps.
• Adds setup steps for test/build prerequisites (device.properties, IARM headers/stubs) used by the GNOME/libnm proxy build configuration.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (5)

cov_build.sh:16

• The usage example suggests overriding THUNDER_REF, but this script doesn’t read THUNDER_REF (it only uses TOOLCHAIN_FILE/WORKSPACE/NETWORKMANAGER_DIR). This is misleading documentation for users running the script standalone.

# Override any of the environment variables below on the command line, e.g.:
#   THUNDER_REF=R4.4.3 ./cov_build.sh

build_dependencies.sh:96

• When the repo directory already exists, git fetch origin "${ref}" only updates FETCH_HEAD (it doesn’t reliably create/update a local branch/tag ref). git checkout "${ref}" can therefore fail depending on whether the ref already exists locally. Checking out FETCH_HEAD after fetching avoids this.

    log "Repository ${path} already present; fetching ${ref}"
    git -C "${path}" fetch origin "${ref}"
    git -C "${path}" checkout "${ref}"
  fi

build_dependencies.sh:81

• The || true at the end of the pip install chain will hide genuine installation failures when pip is present, which can lead to later build steps failing with less actionable errors. Since other CI workflows install jsonref unconditionally, it’s better to fail early if installation fails.

if command -v pip >/dev/null 2>&1; then
  pip install --break-system-packages jsonref || pip install jsonref || true
elif command -v pip3 >/dev/null 2>&1; then
  pip3 install --break-system-packages jsonref || pip3 install jsonref || true
else
  echo "pip not found; skipping jsonref install"
fi

.github/workflows/native_full_build.yml:23

• These scripts rely on bash features and re-exec into bash, but running them via sh -x drops xtrace when they exec (so the logs won’t actually be traced). Invoke them with bash directly to preserve -x and avoid any shell-compat surprises.

      - name: native build
        run: |
          sh -x build_dependencies.sh
          sh -x cov_build.sh

build_dependencies.sh:113

• Patch application errors are currently silenced and treated the same as “already applied”, which can let the script continue with an unpatched Thunder checkout and cause harder-to-debug failures later. The script should only ignore the patch when it is already applied; otherwise fail with a clear error.

  git apply "${NETWORKMANAGER_DIR}/tests/patches/thunder/SubscribeStub.patch" 2>/dev/null \
    || echo "Patch already applied or not needed"

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

.github/workflows/native_full_build.yml:23

• The workflow runs the scripts via sh -x, but both scripts immediately re-exec under bash. That means the -x tracing is lost (and running under sh is redundant). Prefer invoking bash directly (or set shell: bash) so debug output and bash semantics are consistent from the start.

      - name: native build
        run: |
          sh -x build_dependencies.sh
          sh -x cov_build.sh

build_dependencies.sh:96

• In clone_repo, the update path does git fetch origin <ref> followed by git checkout <ref>. This can fail when <ref> is not already a local branch/tag (e.g., a remote branch name not present locally). Fetching all refs (or tags) and force-checking out the requested ref makes reruns more reliable.

  else
    log "Repository ${path} already present; fetching ${ref}"
    git -C "${path}" fetch origin "${ref}"
    git -C "${path}" checkout "${ref}"
  fi

build_dependencies.sh:114

• The patch application currently treats any git apply failure as "already applied or not needed" by discarding stderr and continuing. This can hide real failures (e.g., patch no longer applies for the selected Thunder ref) and lead to harder-to-diagnose build errors later. Consider explicitly detecting the "already applied" case and failing otherwise.

log "Applying Thunder SubscribeStub patch"
(
  cd "${WORKSPACE}/Thunder"
  git apply "${NETWORKMANAGER_DIR}/tests/patches/thunder/SubscribeStub.patch" 2>/dev/null \
    || echo "Patch already applied or not needed"

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (4)

build_dependencies.sh:96

• When the repo already exists, the function fetches but then just checks out the ref name; this does not guarantee the working tree is updated to the fetched commit (e.g., for branch refs it may remain on an old local branch tip). Update the existing checkout to the remote branch when applicable, otherwise fall back to tag/commit checkout.

  else
    log "Repository ${path} already present; fetching ${ref}"
    git -C "${path}" fetch origin "${ref}"
    git -C "${path}" checkout "${ref}"
  fi

build_dependencies.sh:114

• The patch application currently suppresses all errors and continues. That can hide real failures (e.g., patch drift) and lead to harder-to-diagnose build errors later. Consider making this idempotent (skip only if already applied) but fail fast on genuine apply/check failures.

  git apply "${NETWORKMANAGER_DIR}/tests/patches/thunder/SubscribeStub.patch" 2>/dev/null \
    || echo "Patch already applied or not needed"

.github/workflows/native_full_build.yml:23

• These scripts are bash scripts and intentionally re-exec under bash when started via sh, but re-execing drops -x tracing. Running them via bash -x here preserves tracing and avoids the extra re-exec step.

          sh -x build_dependencies.sh
          sh -x cov_build.sh

cov_build.sh:16

• The usage example suggests overriding THUNDER_REF, but this script doesn't read THUNDER_REF. This can mislead users trying to customize the build; use an environment variable that cov_build.sh actually consumes (e.g., TOOLCHAIN_FILE, WORKSPACE, NETWORKMANAGER_DIR).

# Override any of the environment variables below on the command line, e.g.:
#   THUNDER_REF=R4.4.3 ./cov_build.sh

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

build_dependencies.sh:157

• With set -euo pipefail, this command substitution will exit the script early if find errors (e.g., when ${INSTALL_DIR}/include is missing), so the more helpful error message below never runs. Consider making the find|head pipeline non-fatal and let the explicit validation handle the error path.

IFACE_DIR="$(find "${INSTALL_DIR}/include" -maxdepth 2 -name "interfaces" -type d | head -1)"

.github/workflows/native_full_build.yml:23

• The scripts re-exec under bash when invoked via sh, so sh -x ... will not keep xtrace enabled for the actual bash execution. Invoke them with bash -x (or execute directly) to get consistent tracing and avoid depending on sh.

          sh -x build_dependencies.sh
          sh -x cov_build.sh

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (4)

cov_build.sh:54

• NM_CXX_FLAGS uses --include for forced includes, but GCC/Clang expect -include <file>. With --include the compiler will typically error on an unrecognized option, breaking the build.

NM_CXX_FLAGS=" -fprofile-arcs -ftest-coverage \
-I ${NETWORKMANAGER_DIR}/tests/headers \
-I ${NETWORKMANAGER_DIR}/tests/headers/rdk/iarmbus \
-I ${NETWORKMANAGER_DIR}/tests/headers/rdk/iarmmgrs-hal \
--include ${NETWORKMANAGER_DIR}/tests/mocks/Iarm.h \
--include ${NETWORKMANAGER_DIR}/tests/mocks/mfrMgr.h "

build_dependencies.sh:97

• When the repo already exists, git fetch origin "${ref}" only fetches into FETCH_HEAD and may not create/update a local ref named ${ref} (especially for tags like R4.4.3). The subsequent git checkout "${ref}" can fail on reruns in the same workspace.

  else
    log "Repository ${path} already present; fetching ${ref}"
    git -C "${path}" fetch origin "${ref}"
    git -C "${path}" checkout "${ref}"
  fi

build_dependencies.sh:81

• The pip/pip3 install is always treated as optional (|| true), but existing CI workflows install jsonref as a required step. If installation fails, continuing will likely cause later failures with a less clear root cause; it’s better to fail fast here.

if command -v pip >/dev/null 2>&1; then
  pip install --break-system-packages jsonref || pip install jsonref || true
elif command -v pip3 >/dev/null 2>&1; then
  pip3 install --break-system-packages jsonref || pip3 install jsonref || true
else

.github/workflows/native_full_build.yml:26

• These scripts intentionally rely on bash and re-exec under bash when invoked via sh, but running them as sh -x means xtrace won’t carry across the exec bash ... and you won’t actually get the expected debug output. Invoking them directly with bash -x avoids the extra shell hop and preserves tracing.

        run: |
          sh -x build_dependencies.sh
          sh -x cov_build.sh