-
-
Notifications
You must be signed in to change notification settings - Fork 781
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix/windowscompile #2016
base: main
Are you sure you want to change the base?
Fix/windowscompile #2016
Conversation
… for probe_skip and improve memory management in FTDI functions
if (probe_skip) { | ||
free((char*)probe_skip); | ||
probe_skip = NULL; | ||
} |
There was a problem hiding this comment.
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.
#pragma GCC diagnostic push | ||
#pragma GCC diagnostic ignored "-Wcast-qual" |
There was a problem hiding this comment.
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.
@@ -149,11 +149,22 @@ int ftdi_read_data(struct ftdi_context *ftdi, unsigned char *buf, const int size | |||
|
|||
int ftdi_write_data(struct ftdi_context *ftdi, const unsigned char *buf, int size) | |||
{ | |||
(void)ftdi; | |||
DWORD bytes_written; | |||
if (FT_Write(ftdi_handle, (unsigned char *)buf, size, &bytes_written) != FT_OK) |
There was a problem hiding this comment.
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.
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