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.

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(usb): Fix -Wincompatible-pointer-types compiler error #2795

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mlouielu
Copy link

@mlouielu mlouielu commented Jan 24, 2025

Fix #2794, where get_keyboard_report(len) didn't convert len from int32_t * to size_t *.

PR check-list

  • Branch has a clean commit history
  • Additional tests are included, if changing behaviors/core code that is testable.
  • Proper Copyright + License headers added to applicable files (Generally, we stick to "The ZMK Contributors" for copyrights to help avoid churn when files get edited)
  • Pre-commit used to check formatting of files, commit messages, etc.
  • Includes any necessary documentation changes.

Fix zmkfirmware#2794, where `get_keyboard_report(len)` didn't convert
`len` from `int32_t *` to `size_t *`.
@mlouielu mlouielu requested a review from a team as a code owner January 24, 2025 17:07
@@ -89,7 +89,7 @@ static int get_report_cb(const struct device *dev, struct usb_setup_packet *setu
case HID_REPORT_TYPE_INPUT:
switch (setup->wValue & HID_GET_REPORT_ID_MASK) {
case ZMK_HID_REPORT_ID_KEYBOARD: {
*data = get_keyboard_report(len);
*data = get_keyboard_report((size_t *)len);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This doesn't actually fix the issue that the compiler is warning you about. It just says, "I know what I'm doing", so the compiler stops warning about it.

len is still a pointer to int32_t. Casting it to a pointer to size_t means that get_keyboard_report() is going to write a size_t to the memory that is actually an int32_t. On a platform such as native_posix_64 where size_t is uint64_t, it will write a 64-bit value to the given address, but the variable is only 32 bits, so it clobbers some memory after it.

Some safe alternatives:

  • Change get_keyboard_report() to also use an int32_t for its output parameter (requires also updating any other places that call the function to use int32_t as well).
  • Create a new size_t variable here, pass its address to get_keyboard_report(), then assign its value to len.

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.

Having -Wincompatible-pointer-types when building with CONFIG_ZMK_USB=y
2 participants