7
31
Fork
You've already forked blackmagic
23

Fix/windowscompile #2016

Open
zhangmx wants to merge 2 commits from zhangmx/fix/windowscompile into main
pull from: zhangmx/fix/windowscompile
merge into: blackmagic-debug:main
blackmagic-debug:main
blackmagic-debug:fix/pre-bmp-v3-cross-file-cleanup
blackmagic-debug:feature/bmda-remote-comms
blackmagic-debug:ALTracer/feature/aarch64-ident
blackmagic-debug:feature/better-meson-optimisation-handling
blackmagic-debug:feature/am335x-support
blackmagic-debug:feature/esp32-c3-support
blackmagic-debug:feature/cortex-ar-software-breakpoints
blackmagic-debug:feature/unit-testing
blackmagic-debug:feature/windows-usb-serial-interface-naming
blackmagic-debug:fix/bmp-external-spi
blackmagic-debug:ALTracer/feature/bluepillplus-platform
blackmagic-debug:ALTracer/feature/at32f43x-unrdp
blackmagic-debug:feature/const-correctness
blackmagic-debug:ALTracer/feature/fault_handlers
blackmagic-debug:ALTracer/feature/hazard3-ice40-support
blackmagic-debug:fix/ci-cleanup
blackmagic-debug:ALTracer/fix/gdb-10-12-thread
blackmagic-debug:ALTracer/feature/blackpill-f4-adc
blackmagic-debug:ALTracer/fix/cortex-desc-allocfail-report
blackmagic-debug:ALTracer/feature/spi-perf
blackmagic-debug:ALTracer/feature/calibrate_swd
blackmagic-debug:ALTracer/feature/blank-check
blackmagic-debug:feature/avr
blackmagic-debug:v2.0
blackmagic-debug:v1.9
blackmagic-debug:v1.10
blackmagic-debug:v1.8
zhangmx commented 2024年12月11日 10:12:47 +01:00 (Migrated from github.com)
Copy link

Detailed description

During the compilation of the project in a Windows MSYS2 environment, I encountered two warning messages and made some adjustments to address these issues. One of the problems was related to the const qualifier, which caused the compilation failure. I resolved this issue by using a type cast to ensure the pointer could be passed correctly.

Your checklist for this pull request

<!-- Filling this template is mandatory --> ## Detailed description During the compilation of the project in a Windows MSYS2 environment, I encountered two warning messages and made some adjustments to address these issues. One of the problems was related to the const qualifier, which caused the compilation failure. I resolved this issue by using a type cast to ensure the pointer could be passed correctly. <!-- Explain the **details** for making this change. * Is a new feature implemented? * What existing problem(s) does the pull request solve? * How does the pull request solve these problems? Please provide enough information so that others can review your pull request. Information embedded in the description part of the commits doesn't count. --> ## Your checklist for this pull request * [x] I've read the [Code of Conduct](https://github.com/blackmagic-debug/blackmagic/blob/main/CODE_OF_CONDUCT.md) * [x] I've read the [guidelines for contributing](https://github.com/blackmagic-debug/blackmagic/blob/main/CONTRIBUTING.md) to this repository * [x] It builds for hardware native (see [Building the firmware](https://github.com/blackmagic-debug/blackmagic?tab=readme-ov-file#building-black-magic-debug-firmware)) * [x] It builds as BMDA (see [Building the BMDA](https://github.com/blackmagic-debug/blackmagic?tab=readme-ov-file#building-black-magic-debug-app)) * [x] I've tested it to the best of my ability * [x] My commit messages provide a useful short description of what the commits do

This is good, and is the correct fix, but please limit where the warning is turned off to just the free() lines where the cast occurs. We want that warning on for the rest of the function as it helps find const correctness issues and prevent mistakes.

This is good, and is the correct fix, but please limit where the warning is turned off to just the `free()` lines where the cast occurs. We want that warning on for the rest of the function as it helps find `const` correctness issues and prevent mistakes.

There's a couple of things here - the first is that this code can be simplified - there's no point checking probe_skip is not NULL as free(NULL); is well defined to be a noop. The second is that because this function immediately returns after, there's no need to NULL probe_skip as there's no potential for use-after-free. Finally, the cast to void * is fine as such, but the call to free(), due to free() not being const-correct, needs wrapping in the

#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wcast-qual"

you've inserted at the top of this function.

There's a couple of things here - the first is that this code can be simplified - there's no point checking `probe_skip` is not `NULL` as `free(NULL);` is well defined to be a noop. The second is that because this function immediately returns after, there's no need to `NULL` `probe_skip` as there's no potential for use-after-free. Finally, the cast to `void *` is fine as such, but the call to `free()`, due to `free()` not being `const`-correct, needs wrapping in the ```c #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wcast-qual" ``` you've inserted at the top of this function.
@ -157,0 +164,4 @@
}
free(temp_buf);
return bytes_written;

This line should only need wrapping with

#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wcast-qual"
...
#pragma GCC diagnostic pop

Not sure the rest of this change should be necessary and it massively pessimises performance if it is, so we want to avoid it if possible.

This line should only need wrapping with ```c #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wcast-qual" ... #pragma GCC diagnostic pop ``` Not sure the rest of this change should be necessary and it massively pessimises performance if it is, so we want to avoid it if possible.
This pull request has changes conflicting with the target branch.
  • src/platforms/hosted/bmp_libusb.c
  • src/platforms/hosted/windows/ftdi.c
View command line instructions

Manual merge helper

Use this merge commit message when completing the merge manually.

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin zhangmx/fix/windowscompile:zhangmx/fix/windowscompile
git switch zhangmx/fix/windowscompile
Sign in to join this conversation.
No reviewers
Labels
Clear labels
BMD App
Black Magic Debug App (aka. PC hosted) (not firmware)
BMP Firmware
Black Magic Probe Firmware (not PC hosted software)
Bug
Confirmed bug
Build system
Build system
Can't reproduce
Maintainers can't reproduce this problem
CI
Continuous Integration System
Contribution wanted
User contributions welcome
Documentation
Project documentation
Draft
Work in progress draft
Duplicate
This issue or pull request already exists
Enhancement
General project improvement
Feedback wanted
Requires additional submitter feedback
Foreign Host Board
Non Native hardware to runing Black Magic firmware on
GDB
Issue/PR related to GDB
Good first issue
Good for newcommers
HwIssue Mitigation
Solving or mitigating a Hardware issue in Software
Information Needed
Maintainers need more information
NativeHardware
Official Black Magic Debug Hardware
New Host Board
New hardware to run Black Magic firmware on
New Target
New debug target
Off Topic
Something that does not involve the project in any way
Potential Bug
A potential, unconfirmed or very special circumstance bug
Regression
Bug caused by a regression
User Interest Needed
More user interest required before consideration
User Testing Needed
Looking for user testing reports
Won't fix
Outside of the project scope or works as intended
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
blackmagic-debug/blackmagic!2016
Reference in a new issue
blackmagic-debug/blackmagic
No description provided.
Delete branch "zhangmx/fix/windowscompile"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?