Skip to content

Conversation

@zhangmx
Copy link

@zhangmx zhangmx commented Dec 11, 2024

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

Comment on lines +327 to +330
if (probe_skip) {
free((char*)probe_skip);
probe_skip = NULL;
}
Copy link
Member

Choose a reason for hiding this comment

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

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.

Comment on lines +248 to +249
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wcast-qual"
Copy link
Member

Choose a reason for hiding this comment

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

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.

{
(void)ftdi;
DWORD bytes_written;
if (FT_Write(ftdi_handle, (unsigned char *)buf, size, &bytes_written) != FT_OK)
Copy link
Member

Choose a reason for hiding this comment

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

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants