Conversation
a01bf9b to
00f3746
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR performs code cleanup and standardization across the TinyUSB codebase. The changes remove deprecated macros, improve code consistency, and fix minor issues related to header includes and code style.
Key changes:
- Removes deprecated
TU_BIN8()macro usage and replaces with hex literals - Renames internal macros to be public (
_TU_CHECK_MCU→TU_MCU_IS_EQUAL,_OSAL_Q_NAME→OSAL_Q_NAME,_TU_ARGS_APPLY_*→TU_ARGS_APPLY_*) - Reorganizes
<stdio.h>includes to centralize them in appropriate header files - Standardizes conditional checks and type casting for consistency
Reviewed Changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/tusb_option.h | Renames internal macro _TU_CHECK_MCU to public TU_MCU_IS_EQUAL, improves clarity |
| src/portable/ohci/ohci.c | Replaces TU_BIN8(111) with hex literal 0x7 |
| src/portable/ehci/ehci.c | Replaces TU_BIN8() macros with hex literals for interrupt masks |
| src/osal/osal_freertos.h | Renames _OSAL_Q_NAME to OSAL_Q_NAME, changes !TUP_MCU_MULTIPLE_CORE to explicit == 0 comparison, adds explicit casts |
| src/common/tusb_verify.h | Replaces tu_printf() with TU_LOG1() for assert messages, removes direct stdio.h include |
| src/common/tusb_types.h | Removes unnecessary 'u' suffix from shift expressions, adds space after cast operator |
| src/common/tusb_debug.h | Moves stdio.h include to the default case (not custom printf) |
| src/common/tusb_compiler.h | Renames internal _TU_ARGS_APPLY_* macros to public TU_ARGS_APPLY_*, fixes comment typo, improves comment clarity |
| src/common/tusb_common.h | Removes stdio.h include, casts memset/memcpy results to void, makes tu_log2 condition explicit, removes TU_BIN8 macro definitions, consolidates function braces |
| src/class/net/ncm_device.c | Removes redundant standard header includes |
| src/class/audio/audio.h | Removes unnecessary cast and 'u' suffix from shift expressions in enum |
| hw/bsp/board_api.h | Adds explicit cast for nibble extraction |
| examples/host/msc_file_explorer/src/msc_app.h | Adds stdio.h include |
| examples/host/msc_file_explorer/src/main.c | Removes stdio.h include (moved to header) |
| examples/host/hid_controller/src/app.h | Adds stdio.h include |
| examples/host/cdc_msc_hid_freertos/src/msc_app.c | Adds stdio.h include |
| examples/host/cdc_msc_hid_freertos/src/app.h | Adds stdio.h include |
| examples/host/cdc_msc_hid/src/msc_app.c | Adds stdio.h include |
| examples/host/cdc_msc_hid/src/hid_app.c | Removes blank line |
| examples/host/cdc_msc_hid/src/app.h | Adds stdio.h include |
| examples/device/cdc_msc/src/main.c | Changes !btn_prev to explicit (btn_prev == 0) comparison |
| examples/device/audio_4_channel_mic/src/usb_descriptors.c | Adds static qualifier, improves brace placement, adds explicit cast |
| .github/workflows/build_util.yml | Extracts inline expression to TOOLCHAIN environment variable for better readability |
| p_qhd->int_smask = (interval == 1) ? TU_BIN8(11111111) : | ||
| (interval == 2) ? TU_BIN8(10101010): TU_BIN8(01000100); | ||
| p_qhd->int_smask = (interval == 1) ? 0xff : // 0b11111111 | ||
| (interval == 2) ? 0xaa /* 0b10101010 */ : 0x44 /* 01000100 */; |
There was a problem hiding this comment.
Binary representation comment is missing '0b' prefix for the last value. Should be '0b01000100' for consistency with the other binary comments on this line and line 922.
| (interval == 2) ? 0xaa /* 0b10101010 */ : 0x44 /* 01000100 */; | |
| (interval == 2) ? 0xaa /* 0b10101010 */ : 0x44 /* 0b01000100 */; |
| #define _TU_ARGS_APPLY_EXPAND_6(_X, _a1, _a2, _a3, _a4, _a5, _a6) _X(_a1), _TU_ARGS_APPLY_EXPAND_5(_X, _a2, _a3, _a4, _a5, _a6) | ||
| #define _TU_ARGS_APPLY_EXPAND_7(_X, _a1, _a2, _a3, _a4, _a5, _a6, _a7) _X(_a1), _TU_ARGS_APPLY_EXPAND_6(_X, _a2, _a3, _a4, _a5, _a6, _a7) | ||
| #define _TU_ARGS_APPLY_EXPAND_8(_X, _a1, _a2, _a3, _a4, _a5, _a6, _a7, _a8) _X(_a1), _TU_ARGS_APPLY_EXPAND_7(_X, _a2, _a3, _a4, _a5, _a6, _a7, _a8) | ||
| // Apply a macro X to each of the arguments with a selected separation/delimiter |
There was a problem hiding this comment.
Grammatical error: 'with a selected' should be 'with a separator' to match the actual functionality. The macro uses a separator/delimiter parameter, not a 'selected' one.
| // Apply a macro X to each of the arguments with a selected separation/delimiter | |
| // Apply a macro X to each of the arguments with a separator/delimiter |
💡 Codex Reviewtinyusb/src/common/tusb_common.h Lines 74 to 79 in a01bf9b Dropping ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
| TU_ATTR_ALWAYS_INLINE static inline | ||
| void tu_edpt_stream_read_xfer_complete(tu_edpt_stream_t* s, uint32_t xferred_bytes) { | ||
| if (tu_fifo_depth(&s->ff)) { | ||
| if (0 != tu_fifo_depth(&s->ff)) { |
Check warning
Code scanning / C-STAT
The operands `0' and `tu_fifo_depth(&s->ff)' have essential type categories signed 8-bit int and unsigned 16-bit int, which do not match
| TU_ATTR_ALWAYS_INLINE static inline | ||
| void tu_edpt_stream_read_xfer_complete_with_buf(tu_edpt_stream_t* s, const void * buf, uint32_t xferred_bytes) { | ||
| if (tu_fifo_depth(&s->ff)) { | ||
| if (0 != tu_fifo_depth(&s->ff)) { |
Check warning
Code scanning / C-STAT
The operands `0' and `tu_fifo_depth(&s->ff)' have essential type categories signed 8-bit int and unsigned 16-bit int, which do not match
|
|
||
| if (dwc2->gsnpsid >= DWC2_CORE_REV_3_00a) { | ||
| if(dwc2->epout[0].doepctl & DOEPCTL_EPENA) { | ||
| if(0 != (dwc2->epout[0].doepctl & DOEPCTL_EPENA)) { |
There was a problem hiding this comment.
@hathach I don't like adding a redundant 0 != into the condition, can we change the static analyzer behavior ?
I think it's better to change uint32_t board_button_read(void); to bool.
There was a problem hiding this comment.
yeah, It is rather annoying, maybe we can suppress that rule. let me check
There was a problem hiding this comment.
Some make sense to change to != 0 though like checking on number. For flag with and/or it is indeed unneccessary.
f7db39c to
47650d4
Compare
47650d4 to
94bb01d
Compare
94bb01d to
778c1dc
Compare
There was a problem hiding this comment.
C-STAT found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
778c1dc to
8979af3
Compare
93a7ca0 to
454f92c
Compare
| (void) tud_msc_set_sense(lun, SCSI_SENSE_ILLEGAL_REQUEST, 0x20, 0x00); | ||
|
|
||
| return -1; | ||
| return -1; // stall/failed command request; |
Check notice
Code scanning / CodeQL
Commented-out code Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
To fix the problem, remove the ambiguous comment on line 246, // stall/failed command request;, in examples/device/cdc_msc/src/msc_disk.c. This will help keep the code clean and free of confusing, commented-out code. No further action is required—simply delete the line containing the comment. No imports, variables, or other code changes are needed; just remove the comment.
| @@ -243,7 +243,6 @@ | ||
| // Set Sense = Invalid Command Operation | ||
| (void) tud_msc_set_sense(lun, SCSI_SENSE_ILLEGAL_REQUEST, 0x20, 0x00); | ||
|
|
||
| return -1; // stall/failed command request; | ||
| } | ||
| return -1; } | ||
|
|
||
| #endif |
fix more alerts disable IAR CStat since pvs-studio check is better integrated with clion
454f92c to
1f04fe7
Compare
There was a problem hiding this comment.
Code Health Improved
(2 files improve in Code Health)
Gates Failed
Enforce advisory code health rules
(1 file with Complex Method)
Gates Passed
3 Quality Gates Passed
See analysis details in CodeScene
Reason for failure
| Enforce advisory code health rules | Violations | Code Health Impact | |
|---|---|---|---|
| video_device.c | 1 advisory rule | 4.06 → 4.05 | Suppress |
View Improvements
| File | Code Health Impact | Categories Improved |
|---|---|---|
| usbd.c | 5.14 → 5.15 | Complex Method |
| dcd_dwc2.c | 6.65 → 6.70 | Complex Method, Overall Code Complexity |
Quality Gate Profile: Clean Code Collective
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.
|



fixed alerts found by pvs-studio