-
Notifications
You must be signed in to change notification settings - Fork 8
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
!warnings:ARM compiler - fixed code vs inconsisting with standart C, … #44
base: master
Are you sure you want to change the base?
Conversation
…that cause warnings pointer type mis-mutch, packed vs unpacked pointer type mis-mutch, !no binary const - C standart not provide binary (0b)constants, so ARM compiler fail on it. * remove code with goto
} else { | ||
/* Search for the UTF-8 string and convert it to UTF-16 */ | ||
const uint8_t *utf8_str; | ||
utf8_str = search_utf8_string(dev, arg->setup->wIndex, index - 1); | ||
used = (utf8_str == NULL) ? -1 : | ||
utf8_to_utf16(utf8_str, sd->wData, max_count); | ||
utf8_to_utf16(utf8_str, (uint16_t*)sd->wData, max_count); |
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.
https://github.com/insane-adding-machines/unicore-mx/blob/master/include/unicore-mx/usb/usbstd.h#L257
sd->wData
is already uint16_t
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.
No, that is not: sd->wData is of type "packed" uint16_t*, since it got from packed struct.
maybe there should be some comments. But , i guess, the temporary buffer preserve.buf - always aligned, so better to dropoff "packed" attribute, instead rewrite utf8_to_utf16 and search_utf8_string capable work with unaligned utf16 strings.
if ((setup_data->bmRequestType & USB_REQ_TYPE_TYPE) != USB_REQ_TYPE_STANDARD) { | ||
goto stall; | ||
} | ||
if ((setup_data->bmRequestType & USB_REQ_TYPE_TYPE) == USB_REQ_TYPE_STANDARD) { |
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.
Code style: 👎 from me.
@@ -720,7 +720,7 @@ static void _control_data_callback(usbd_device *dev, | |||
return; | |||
} | |||
|
|||
usbd_control_transfer_callback callback = transfer->user_data; | |||
usbd_control_transfer_callback callback = (usbd_control_transfer_callback)transfer->user_data; |
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.
C standard says: void * will automatically cast to any other type.
and this style is more maintanable.
see: http://stackoverflow.com/a/605858/1500988
note to myself: place link to C standard text.
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.
But with some stricts - you can assign any pointer TO void* ,
but if you want assign some* to typed* - need casts to destination type
@@ -897,7 +897,7 @@ void *usbd_urb_get_buffer_pointer(usbd_device *dev, usbd_urb *urb, size_t len) | |||
return transfer->buffer; | |||
} | |||
|
|||
return transfer->buffer + transfer->transferred; | |||
return (void*)((uintptr_t)transfer->buffer + transfer->transferred); |
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.
isnt transfer->buffer + transfer->transferred
pointer arithmetic (valid in c).
what is the reason of writing it in a more obsured way?
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.
void* have no information about what entry size is. So yes - compiler is right, when error on such a trick.
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.
You point is valid.
GCC treat void * pointer same as char *, but that is non-standard behaviour.
@@ -208,7 +208,7 @@ static int utf8_to_utf16(const uint8_t *utf8, uint16_t *utf16, | |||
|
|||
/* trailing bytes payload */ | |||
while (trailing_bytes_utf8-- > 0) { | |||
utf32_ch = (utf32_ch << 6) | (utf8[i++] & 0b00111111); | |||
utf32_ch = (utf32_ch << 6) | (utf8[i++] & 0x3f); | |||
} |
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 was a specific reason (readability imo) to write these specific constant in binary notation.
but hexadecimal notation wont do any improvement either.
Update: (0b binary notation) dont seems to be in C specs, your point is valid.
fixed code vs inconsisting with standart C on Keil ARM compiler, that cause warnings pointer type mis-mutch, packed vs unpacked pointer type mis-mutch,
!no binary const - C standart not provide binary (0b)constants, so ARM compiler fail on it.