Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ 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".
src/class/cdc/cdc_device.h
Outdated
| typedef struct TU_ATTR_PACKED { | ||
| uint8_t rx_persistent : 1; // keep rx fifo data even with bus reset or disconnect | ||
| uint8_t tx_persistent : 1; // keep tx fifo data even with reset or disconnect | ||
| uint8_t tx_overwritabe_if_not_connected : 1; // if not connected, tx fifo can be overwritten | ||
| bool rx_persistent : 1; // keep rx fifo data even with bus reset or disconnect | ||
| bool tx_persistent : 1; // keep tx fifo data even with reset or disconnect | ||
| bool tx_overwritabe_if_not_connected : 1; // if not connected, tx fifo can be overwritten | ||
| } tud_cdc_configure_t; | ||
| TU_VERIFY_STATIC(sizeof(tud_cdc_configure_t) == 1, "size is not correct"); | ||
|
|
||
| #define TUD_CDC_CONFIGURE_DEFAULT() { \ | ||
| .rx_persistent = 0, \ | ||
| .tx_persistent = 0, \ | ||
| .tx_overwritabe_if_not_connected = 1, \ | ||
| .rx_persistent = false, \ | ||
| .tx_persistent = false, \ | ||
| .tx_overwritabe_if_not_connected = false, \ |
There was a problem hiding this comment.
Restore default TX FIFO overwrite behaviour
The default configuration macro now sets tx_overwritabe_if_not_connected to false, whereas previously it was 1 and the driver still documents “Default: is overwritable”. This silently changes the public API’s behaviour so that CDC writes while the terminal is disconnected will stop once the FIFO fills rather than overwriting old data. Existing applications that relied on the previous default to keep emitting data before the host asserts DTR will suddenly stall. Please keep the default value true (or update the code/comment and documentation) to avoid this regression.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull Request Overview
This PR focuses on code quality improvements through static analysis fixes, formatting standardization, and MISRA C compliance. The changes include PVS-Studio warning suppressions, explicit comparisons for clarity, proper type usage (bool instead of uint8_t for flags), void casts for unused return values, and clang-format configuration updates for consistent code style.
Key Changes
- Added PVS-Studio static analysis suppressions and configuration improvements
- Enhanced MISRA C compliance with explicit comparisons and better code structure
- Improved type safety by changing flag variables from uint8_t to bool
- Updated clang-format configuration for stricter code formatting standards
Reviewed Changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/portable/ehci/ehci.c | Fixed comment formatting for binary literal |
| src/osal/osal_pico.h | Simplified semaphore post return and added void cast for FIFO clear |
| src/common/tusb_verify.h | Added 'u' suffix to literal for type consistency |
| src/common/tusb_private.h | Added 'u' suffix to literals for explicit unsigned comparison |
| src/common/tusb_debug.h | Added void casts to suppress unused return value warnings (with one issue) |
| src/common/tusb_compiler.h | Minor comment simplification |
| src/class/msc/msc_host.c | Added PVS-Studio suppression comments for memcpy calls |
| src/class/msc/msc_device.c | Extensive MISRA compliance improvements: bool types, explicit comparisons, TU_ATTR_ALWAYS_INLINE, empty else branches, scope braces |
| src/class/cdc/cdc_device.h | Changed bitfield types to bool (with default value issue) |
| src/class/cdc/cdc_device.c | Added explicit comparisons and const qualifiers |
| hw/bsp/rp2040/family.c | Added __nop() in delay loop for clarity and compiler safety |
| hw/bsp/imxrt/family.cmake | Improved CMake string escaping using raw strings |
| examples/device/net_lwip_webserver/src/usb_descriptors.c | Comprehensive formatting improvements and added STRID_COUNT enum |
| examples/device/cdc_msc/src/main.c | Formatting improvements and explicit comparisons |
| .clang-format | Added stricter formatting rules including QualifierAlignment |
| .PVS-Studio/.pvsconfig | Improved path exclusions and suppression rules |
| .github/copilot-instructions.md | Added PVS-Studio analysis documentation |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
# Conflicts: # src/class/cdc/cdc_device.h # src/class/msc/msc_device.c # src/common/tusb_debug.h
There was a problem hiding this comment.
Code Health Improved
(1 files improve in Code Health)
Gates Failed
Prevent hotspot decline
(2 hotspots with Complex Method)
Enforce advisory code health rules
(2 files with Complex Method)
Gates Passed
2 Quality Gates Passed
See analysis details in CodeScene
Reason for failure
| Prevent hotspot decline | Violations | Code Health Impact | |
|---|---|---|---|
| msc_device.c | 1 rule in this hotspot | 6.26 → 6.25 | Suppress |
| audio_device.c | 1 rule in this hotspot | 3.44 → 3.44 | Suppress |
| Enforce advisory code health rules | Violations | Code Health Impact | |
|---|---|---|---|
| msc_device.c | 1 advisory rule | 6.26 → 6.25 | Suppress |
| audio_device.c | 1 advisory rule | 3.44 → 3.44 | Suppress |
View Improvements
| File | Code Health Impact | Categories Improved |
|---|---|---|
| tusb_fifo.c | 7.66 → 7.90 | Code Duplication |
Quality Gate Profile: Clean Code Collective
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.
|




No description provided.