Conversation
There was a problem hiding this comment.
Pull Request Overview
This pull request adds static analysis support and addresses warnings identified by IAR C-STAT and PVS-Studio analyzers. The changes improve code quality through better type safety, proper variable scoping, and coding standard compliance.
Key changes:
- Added IAR C-STAT integration with CMake build system via
-DIAR_CSTAT=1flag - Added PVS-Studio CI workflow for automated static analysis
- Fixed type safety issues by adding unsigned suffixes to integer literals
Reviewed Changes
Copilot reviewed 58 out of 58 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/tusb_option.h | Added unsigned suffixes to speed mode constants |
| src/tusb.c | Added braces to single-statement if/for blocks |
| src/portable/synopsys/dwc2/dcd_dwc2.c | Added unsigned suffix to bitshift operations; added comment explaining removed code |
| src/osal/osal_none.h | Added parameter names to function pointer typedefs |
| src/osal/osal.h | Added parameter names to function pointer typedef; reformatted comments |
| src/device/dcd.h | Added parameter name to function pointer typedef |
| src/common/tusb_types.h | Added unsigned suffixes to enum values |
| src/common/tusb_debug.h | Added braces to single-statement blocks; removed TODO comment |
| src/common/tusb_common.h | Added unsigned suffix to comparison value |
| hw/bsp/stm32n6/family.c | Made UART handle static |
| hw/bsp/stm32h7rs/family.c | Made UART handle static |
| hw/bsp/stm32h7/family.c | Made UART handle static; added unsigned suffix to divisor |
| hw/bsp/stm32h7/boards/stm32h743eval/board.h | Changed LED active state from 1 to 0 |
| hw/bsp/stm32f7/family.c | Made UART handle static |
| hw/bsp/stm32f4/family.c | Made UART handle static |
| hw/bsp/samd5x_e5x/family.c | Replaced magic number with sizeof |
| hw/bsp/family_support.cmake | Enabled IAR C-STAT integration; reformatted properties |
| hw/bsp/board_api.h | Added braces to if statement; changed char array to unsigned char |
| hw/bsp/board.c | Removed redundant cast; removed commented-out code; added unsigned suffixes |
| examples/dual/host_info_to_device_cdc/src/usb_descriptors.c | Made USB descriptors static |
| examples/dual/host_hid_to_device_cdc/src/usb_descriptors.c | Made USB descriptors static |
| examples/device/webusb_serial/src/usb_descriptors.c | Made USB descriptors static |
| examples/device/video_capture_2ch/src/usb_descriptors.c | Made USB descriptors static |
| examples/device/video_capture/src/usb_descriptors.c | Made USB descriptors static |
| examples/device/usbtmc/src/usb_descriptors.c | Made USB descriptors static; added missing callback function |
| examples/device/uac2_speaker_fb/src/usb_descriptors.c | Made USB descriptors static |
| examples/device/uac2_headset/src/usb_descriptors.c | Made USB descriptors static |
| examples/device/net_lwip_webserver/src/usb_descriptors.c | Made USB descriptor static |
| examples/device/mtp/src/usb_descriptors.c | Made USB descriptors static |
| examples/device/msc_dual_lun/src/usb_descriptors.c | Made USB descriptors static |
| examples/device/midi_test_freertos/src/usb_descriptors.c | Made USB descriptors static |
| examples/device/midi_test/src/usb_descriptors.c | Made USB descriptors static |
| examples/device/hid_multiple_interface/src/usb_descriptors.c | Made USB descriptors static |
| examples/device/hid_generic_inout/src/usb_descriptors.c | Made USB descriptors static |
| examples/device/hid_composite_freertos/src/usb_descriptors.c | Made USB descriptors static |
| examples/device/hid_composite/src/usb_descriptors.c | Made USB descriptors static |
| examples/device/hid_boot_interface/src/usb_descriptors.c | Made USB descriptors static |
| examples/device/dynamic_configuration/src/usb_descriptors.c | Made USB descriptors static |
| examples/device/dynamic_configuration/src/msc_disk.c | Added static keyword to disk buffer |
| examples/device/dfu_runtime/src/usb_descriptors.c | Made USB descriptors static |
| examples/device/dfu/src/usb_descriptors.c | Made USB descriptors static |
| examples/device/cdc_uac2/src/usb_descriptors.c | Made USB descriptors static |
| examples/device/cdc_msc_freertos/src/usb_descriptors.c | Made USB descriptors static |
| examples/device/cdc_msc_freertos/src/msc_disk.c | Added static keyword to disk buffer |
| examples/device/cdc_msc/src/usb_descriptors.c | Made USB descriptors static |
| examples/device/cdc_msc/src/msc_disk.c | Added static keyword to disk buffer; added braces to if statement |
| examples/device/cdc_msc/src/main.c | Added blink_enable flag to control LED behavior when terminal connects |
| examples/device/cdc_dual_ports/src/usb_descriptors.c | Made USB descriptors static |
| examples/device/audio_test_multi_rate/src/usb_descriptors.c | Made USB descriptor static |
| examples/device/audio_test_freertos/src/usb_descriptors.c | Made USB descriptor static |
| examples/device/audio_test/src/usb_descriptors.c | Made USB descriptor static |
| examples/device/audio_4_channel_mic_freertos/src/usb_descriptors.c | Made USB descriptor static |
| examples/device/audio_4_channel_mic/src/usb_descriptors.c | Made USB descriptor static |
| examples/build_system/cmake/toolchain/cstat_sel_checks.txt | Added MISRA C:2012 checks configuration file |
| examples/build_system/cmake/toolchain/arm_iar.cmake | Added IAR C-STAT analyzer configuration |
| .github/workflows/static_analysis.yml | Added new workflow for PVS-Studio static analysis |
| .github/workflows/build_util.yml | Added PVS-Studio analysis support |
| .github/workflows/build.yml | Updated download-artifact action from v4 to v5 |
| uint32_t dcfg = dwc2->dcfg & ~DCFG_DSPD_Msk; | ||
| if (is_highspeed) { | ||
| dcfg |= DCFG_DSPD_HS << DCFG_DSPD_Pos; | ||
| // dcfg Highspeed's mask is 0 |
There was a problem hiding this comment.
This comment is misleading and unclear. If the mask is 0, explain why the assignment operation on line 404 was removed, or clarify what 'mask is 0' means in this context. Consider: 'High speed configuration uses a mask value of 0, so no explicit assignment is needed' or document the register behavior.
| // dcfg Highspeed's mask is 0 | |
| // High-speed configuration: DCFG_DSPD mask is 0, so no explicit assignment is needed. | |
| // The register defaults to high-speed mode per DWC2 specification. |
src/osal/osal.h
Outdated
Check notice
Code scanning / CodeQL
Commented-out code Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 5 months ago
To fix the issue, the large C comment block containing function signatures should be removed from src/osal/osal.h. If the content is valuable as reference documentation, it should instead be placed in a markdown documentation file, or be formatted as Doxygen/compatible documentation in the code to avoid being mistaken for code. However, per the instructions to remove or reinstate and without assuming documentation systems not shown, simply remove the entire block between line 74 and line 97 inclusive. No other changes are required in this header file.
| @@ -71,32 +71,9 @@ | ||
| #error OS is not supported yet | ||
| #endif | ||
|
|
||
| /*-------------------------------------------------------------------- | ||
| OSAL Porting API | ||
| Should be implemented as static inline function in osal_port.h header | ||
| void osal_spin_init(osal_spinlock_t *ctx); | ||
| void osal_spin_lock(osal_spinlock_t *ctx, bool in_isr) | ||
| void osal_spin_unlock(osal_spinlock_t *ctx, bool in_isr); | ||
|
|
||
| osal_semaphore_t osal_semaphore_create(osal_semaphore_def_t* semdef); | ||
| bool osal_semaphore_delete(osal_semaphore_t semd_hdl); | ||
| bool osal_semaphore_post(osal_semaphore_t sem_hdl, bool in_isr); | ||
| bool osal_semaphore_wait(osal_semaphore_t sem_hdl, uint32_t msec); | ||
| void osal_semaphore_reset(osal_semaphore_t sem_hdl); | ||
|
|
||
| osal_mutex_t osal_mutex_create(osal_mutex_def_t* mdef); | ||
| bool osal_mutex_delete(osal_mutex_t mutex_hdl) | ||
| bool osal_mutex_lock (osal_mutex_t sem_hdl, uint32_t msec); | ||
| bool osal_mutex_unlock(osal_mutex_t mutex_hdl); | ||
|
|
||
| osal_queue_t osal_queue_create(osal_queue_def_t* qdef); | ||
| bool osal_queue_delete(osal_queue_t qhdl); | ||
| bool osal_queue_receive(osal_queue_t qhdl, void* data, uint32_t msec); | ||
| bool osal_queue_send(osal_queue_t qhdl, void const * data, bool in_isr); | ||
| bool osal_queue_empty(osal_queue_t qhdl); | ||
| --------------------------------------------------------------------------*/ | ||
|
|
||
|
|
||
| #ifdef __cplusplus | ||
| } | ||
| #endif |
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
83e393d to
d507f54
Compare
d507f54 to
d7c4bf1
Compare
| chmod +x toolchain.run | ||
| ./toolchain.run -p ~/cache/${{ inputs.toolchain }}/gnurx -y | ||
| elif [[ ${{ inputs.toolchain }} == arm-iar ]]; then | ||
| wget --progress=dot:giga https://netstorage.iar.com/FileStore/STANDARD/001/003/926/iar-lmsc-tools_1.8_amd64.deb -O ~/cache/${{ inputs.toolchain }}/iar-lmsc-tools.deb |
Check failure
Code scanning / SonarCloud
GitHub Actions should not be vulnerable to script injections High
| run: | | ||
| if [[ ${{ inputs.toolchain }} == arm-iar ]]; then | ||
| sudo apt-get install -y ~/cache/${{ inputs.toolchain }}/cxarm.deb | ||
| sudo dpkg -i ~/cache/${{ inputs.toolchain }}/iar-lmsc-tools.deb |
Check failure
Code scanning / SonarCloud
GitHub Actions should not be vulnerable to script injections High
| if [[ ${{ inputs.toolchain }} == arm-iar ]]; then | ||
| sudo apt-get install -y ~/cache/${{ inputs.toolchain }}/cxarm.deb | ||
| sudo dpkg -i ~/cache/${{ inputs.toolchain }}/iar-lmsc-tools.deb | ||
| sudo apt install -y ~/cache/${{ inputs.toolchain }}/cxarm.deb |
Check failure
Code scanning / SonarCloud
GitHub Actions should not be vulnerable to script injections High
44374ae to
e438199
Compare
e438199 to
42f000d
Compare
|
|
||
| // out of ramdisk | ||
| if (lba >= DISK_BLOCK_NUM) return -1; | ||
| if (lba >= DISK_BLOCK_NUM) { |
Check warning
Code scanning / C-STAT
The operands lba' and DISK_BLOCK_NUM' have essential type categories unsigned 32-bit int and signed 8-bit int, which do not match Warning
|
|
||
| // full speed configuration | ||
| uint8_t const desc_fs_configuration[] = { | ||
| static uint8_t const desc_fs_configuration[] = { |
Check warning
Code scanning / C-STAT
Implicit conversion of `{TUD_CONFIG_DESCRIPTOR,TUD_CONFIG_DESCRIPTOR,((TUD_CONFIG_DESC_LEN+(TUD_CDC_DESC_LEN+TUD_CDC_DESC_LEN+TUD_CDC_DESC_LEN+TUD_CDC_DESC_LEN+TUD_CDC_DESC_LEN+TUD_CDC_DESC_LEN+TUD_CDC_DESC_LEN+TUD_CDC_DESC_LEN+TUD_CDC_DESC_LEN+TUD_CDC_DESC_LEN)+(TUD_MSC_DESC_LEN+TUD_MSC_DESC_LEN+TUD_MSC_DESC_LEN))&TU_U16_LOW),(((TUD_CONFIG_DESC_LEN+(TUD_CDC_DESC_LEN+TUD_CDC_DESC_LEN+TUD_CDC_DESC_LEN+TUD_CDC_DESC_LEN+TUD_CDC_DESC_LEN+TUD_CDC_DESC_LEN+TUD_CDC_DESC_LEN+TUD_CDC_DESC_LEN+TUD_CDC_DESC_LEN+TUD_CDC_DESC_LEN)+(TUD_MSC_DESC_LEN+TUD_MSC_DESC_LEN+TUD_MSC_DESC_LEN))>>TU_U16_HIGH)&TU_U16_HIGH),,TUD_CONFIG_DESCRIPTOR,TUD_CONFIG_DESCRIPTOR,((TU_BIT<<TUD_CONFIG_DESCRIPTOR)|TUD_CONFIG_DESCRIPTOR),(TUD_CONFIG_DESCRIPTOR/TUD_CONFIG_DESCRIPTOR),TUD_CDC_DESCRIPTOR,TUD_CDC_DESCRIPTOR,TUD_CDC_DESCRIPTOR,TUD_CDC_DESCRIPTOR,TUD_CDC_DESCRIPTOR,TUD_CDC_DESCRIPTOR,TUD_CDC_DESCRIPTOR,TUD_CDC_DESCRIPTOR,TUD_CDC_DESCRIPTOR,TUD_CDC_DESCRIPTOR,TUD_CDC_DESCRIPTOR,TUD_CDC_DESCRIPTOR,TUD_CDC_DESCRIPTOR,TUD_CDC_DESCRIPTOR,TUD_CDC_DESCRIPTOR,TUD_CDC_DESCRIPTOR,TUD_CDC_DESCRIPTOR,TUD_CDC_DESCRIPTOR,TUD_CDC_DESCRIPTOR,TUD_CDC_DESCRIPTOR,(TUD_CDC_DESCRIPTOR&TU_U16_LOW),((TUD_CDC_DESCRIPTOR>>TU_U16_HIGH)&TU_U16_HIGH),TUD_CDC_DESCRIPTOR,TUD_CDC_DESCRIPTOR,TUD_CDC_DESCRIPTOR,TUD_CDC_DESCRIPTOR,(TUD_CDC_DESCRIPTOR+TUD_CDC_DESCRIPTOR),TUD_CDC_DESCRIPTOR,TUD_CDC_DESCRIPTOR,TUD_CDC_DESCRIPTOR,TUD_CDC_DESCRIPTOR,TUD_CDC_DESCRIPTOR,TUD_CDC_DESCRIPTOR,TUD_CDC_DESCRIPTOR,TUD_CDC_DESCRIPTOR,(TUD_CDC_DESCRIPTOR+TUD_CDC_DESCRIPTOR),TUD_CDC_DESCRIPTOR,TUD_CDC_DESCRIPTOR,EPNUM_CDC_NOTIF,TUD_CDC_DESCRIPTOR,(TUD_CDC_DESCRIPTOR&TU_U16_LOW),((TUD_CDC_DESCRIPTOR>>TU_U16_HIGH)&TU_U16_HIGH),TUD_CDC_DESCRIPTOR,TUD_CDC_DESCRIPTOR,TUD_CDC_DESCRIPTOR,(TUD_CDC_DESCRIPTOR+TUD_CDC_DESCRIPTOR),TUD_CDC_DESCRIPTOR,TUD_CDC_DESCRIPTOR,TUD_CDC_DESCRIPTOR,TUD_CDC_DESCRIPTOR,TUD_CDC_DESCRIPTOR,TUD_CDC_DESCRIPTOR,TUD_CDC_DESCRIPTOR,TUD_CDC_DESCRIPTOR,EPNUM_CDC_OUT,TUD_CDC_DESCRIPTOR,(TUD_CDC_DESCRIPTOR&TU_U16_LOW),((TUD_CDC_DESCRIPTOR>>TU_U16_HIGH)&TU_U16_HIGH),TUD_CDC_DESCRIPTOR,TUD_CDC_DESCRIPTOR,TUD_CDC_DESCRIPTOR,EPNUM_CDC_IN,TUD_CDC_DESCRIPTOR,(TUD_CDC_DESCRIPTOR&TU_U16_LOW),((TUD_CDC_DESCRIPTOR>>TU_U16_HIGH)&TU_U16_HIGH),TUD_CDC_DESCRIPTOR,TUD_MSC_DESCRIPTOR,TUD_MSC_DESCRIPTOR,,TUD_MSC_DESCRIPTOR,TUD_MSC_DESCRIPTOR,TUD_MSC_DESCRIPTOR,,TUD_MSC_DESCRIPTOR,TUD_MSC_DESCRIPTOR,TUD_MSC_DESCRIPTOR,TUD_MSC_DESCRIPTOR,EPNUM_MSC_OUT,TUD_MSC_DESCRIPTOR,(TUD_MSC_DESCRIPTOR&TU_U16_LOW),((TUD_MSC_DESCRIPTOR>>TU_U16_HIGH)&TU_U16_HIGH),TUD_MSC_DESCRIPTOR,TUD_MSC_DESCRIPTOR,TUD_MSC_DESCRIPTOR,EPNUM_MSC_IN,TUD_MSC_DESCRIPTOR,(TUD_MSC_DESCRIPTOR&TU_U16_LOW),((TUD_MSC_DESCRIPTOR>>TU_U16_HIGH)&TU_U16_HIGH),TUD_MSC_DESCRIPTOR}' from essential type array of "anonymous enum" to different or narrower essential type array of "unsigned 8-bit int" Warning
|
|
add some of mentioned tools in #1973 |




Note IAR CStat has issue when analyzing mtp example, therefore we only run it with cdc_msc example for now