Skip to content
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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix/windowscompile #2016

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5efd652
misc: Fixed windows compile error by -Wcast-qual.Correct pointer type…
zhangmx Dec 11, 2024
23fdf08
misc: Fixed the cast qualification warnings
zhangmx Dec 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions src/platforms/hosted/bmp_libusb.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,8 @@ void stlinkv2_read_serial(libusb_device_descriptor_s *device_descriptor, libusb_
}

#if defined(_WIN32) || defined(__CYGWIN__)
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wcast-qual"
Comment on lines +248 to +249
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.

static probe_info_s *process_ftdi_probe(void)
{
DWORD ftdi_dev_count = 0;
Expand Down Expand Up @@ -285,7 +287,7 @@ static probe_info_s *process_ftdi_probe(void)

if (probe_skip) { // Clean up any previous serial number to skip
use_serial = true;
free((void *)probe_skip);
free((char*)probe_skip);
probe_skip = NULL;
}

Expand Down Expand Up @@ -322,11 +324,14 @@ static probe_info_s *process_ftdi_probe(void)
free(product);
}
}
if (probe_skip)
free((void *)probe_skip);
if (probe_skip) {
free((char*)probe_skip);
probe_skip = NULL;
}
Comment on lines +327 to +330
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.

free(dev_info);
return probe_list;
}
#pragma GCC diagnostic pop
#endif

void orbtrace_read_version(libusb_device *device, libusb_device_handle *handle, char *version, size_t buffer_size)
Expand Down
21 changes: 16 additions & 5 deletions src/platforms/hosted/windows/ftdi.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
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.

return 0;
return bytes_written;
(void)ftdi;
DWORD bytes_written;

unsigned char *temp_buf = (unsigned char *)malloc(size);
if (temp_buf == NULL) {
return 0;
}
memcpy(temp_buf, buf, size);

if (FT_Write(ftdi_handle, temp_buf, size, &bytes_written) != FT_OK) {
free(temp_buf);
return 0;
}

free(temp_buf);
return bytes_written;
}

int ftdi_write_data_set_chunksize(struct ftdi_context *ftdi, unsigned int chunksize)
Expand Down
Loading