Skip to content

fix warnings#3320

Merged
hathach merged 10 commits into
masterfrom
more-sast
Oct 29, 2025
Merged

fix warnings#3320
hathach merged 10 commits into
masterfrom
more-sast

Conversation

@hathach
Copy link
Copy Markdown
Owner

@hathach hathach commented Oct 27, 2025

This pull request introduces improvements to static analysis workflow configuration, compiler warning management, and code hygiene across the project. The main changes include refining the GitHub Actions workflow to run static analysis only on relevant files, restructuring how compiler warnings are set for TinyUSB and example sources, and cleaning up legacy or redundant code. Additionally, there are targeted updates to suppress specific compiler warnings and minor code style improvements.

Workflow and CI improvements:

  • Updated .github/workflows/static_analysis.yml to trigger static analysis only on changes to relevant source and configuration files, added concurrency control to prevent overlapping runs, and refined branch targeting for both push and pull request events.

Compiler warning management:

  • Replaced the legacy family_add_default_example_warnings function in hw/bsp/family_support.cmake with a new implementation in hw/bsp/rp2040/family.cmake. The new version applies warning flags directly to TinyUSB interface sources and example sources, with filtering to exclude third-party libraries, and sets appropriate warning and linker options based on GCC version. [1] [2]
  • Enabled previously commented-out warning flags -Wredundant-decls and -Wmissing-prototypes in hw/bsp/family_support.cmake to strengthen static analysis during builds.

Warning suppression and code hygiene:

  • Simplified and consolidated suppression of conversion and sign-conversion warnings for specific third-party sources in hw/bsp/rp2040/family.cmake, removing redundant or version-specific suppression logic.
  • Added targeted suppression of -Wsign-conversion warnings for src/portable/raspberrypi/pio_usb/hcd_pio_usb.c using GCC diagnostic pragmas, improving build cleanliness without affecting other files.

Miscellaneous improvements:

  • Added exclusions for third-party libraries and test sources in SonarQube analysis configuration (sonar-project.properties) to focus code quality metrics on project-owned code.
  • Made the get_bootsel_button function in hw/bsp/rp2040/family.c static to limit its scope and prevent unintended external linkage.

Copilot AI review requested due to automatic review settings October 27, 2025 05:15
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds Qodana static analysis to the CI workflow and optimizes the static analysis configuration by limiting when it runs and what it analyzes.

Key changes:

  • Restricts static analysis workflow to only run on changes in src/, examples/, and hw/bsp/ directories
  • Adds a new Qodana static analysis job that scans the mimxrt1060_evk board example
  • Excludes external dependencies (lib/**, hw/mcu/**, test/**) from SonarQube analysis

Reviewed Changes

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

File Description
.github/workflows/static_analysis.yml Adds path filters to reduce unnecessary CI runs and introduces a new Qodana scanning job
sonar-project.properties Excludes external dependencies and test directories from SonarQube analysis

Comment thread .github/workflows/static_analysis.yml Outdated
- name: 'Qodana Scan'
uses: JetBrains/qodana-action@v2025.2
with:
args: --image,jetbrains/qodana-clang:2025.2-eap # Community version
Copy link

Copilot AI Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The args parameter uses comma separators ('--image,jetbrains/qodana-clang:2025.2-eap') which is non-standard YAML syntax. According to Qodana action documentation, this should be a proper YAML list. Use standard YAML list syntax: args: | with newline-separated arguments or args: ['--image', 'jetbrains/qodana-clang:2025.2-eap'].

Suggested change
args: --image,jetbrains/qodana-clang:2025.2-eap # Community version
args: ['--image', 'jetbrains/qodana-clang:2025.2-eap'] # Community version

Copilot uses AI. Check for mistakes.
codescene-delta-analysis[bot]

This comment was marked as outdated.

codescene-delta-analysis[bot]

This comment was marked as outdated.

@chatgpt-codex-connector
Copy link
Copy Markdown

💡 Codex Review

- name: Checkout TinyUSB
uses: actions/checkout@v4
with:
ref: ${{ github.event.pull_request.head.sha }} # to check out the actual pull request commit, not the merge commit
fetch-depth: 0 # a full history is required for pull request analysis

P1 Badge Guard Qodana checkout for non‑PR events

The new Qodana job runs for both push and pull_request triggers, but the checkout step unconditionally references github.event.pull_request.head.sha. On push events this object is undefined, so the expression fails before the job starts, causing every qualifying push to master to error out and the whole static-analysis workflow to fail. Add an if: github.event_name == 'pull_request' (or a conditional ref) so that pushes use the default commit SHA and do not crash the job.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

codescene-delta-analysis[bot]

This comment was marked as outdated.

@hathach hathach changed the title adding qodana fix warnings Oct 27, 2025
codescene-delta-analysis[bot]

This comment was marked as outdated.

codescene-delta-analysis[bot]

This comment was marked as outdated.

Comment thread examples/device/cdc_msc/src/usb_descriptors.c Fixed
codescene-delta-analysis[bot]

This comment was marked as outdated.

codescene-delta-analysis[bot]

This comment was marked as outdated.

codescene-delta-analysis[bot]

This comment was marked as outdated.

codescene-delta-analysis[bot]

This comment was marked as outdated.

codescene-delta-analysis[bot]

This comment was marked as outdated.

codescene-delta-analysis[bot]

This comment was marked as outdated.

codescene-delta-analysis[bot]

This comment was marked as outdated.

@sonarqubecloud
Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
1 Security Hotspot

See analysis details on SonarQube Cloud

@hathach hathach merged commit 3ff5415 into master Oct 29, 2025
95 of 96 checks passed
@hathach hathach deleted the more-sast branch October 29, 2025 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants