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

bit field arrangement is not guaranteed by the C standard (cdc.h) #1892

Closed
1 task done
jefftrull opened this issue Feb 10, 2023 · 4 comments · Fixed by #1893
Closed
1 task done

bit field arrangement is not guaranteed by the C standard (cdc.h) #1892

jefftrull opened this issue Feb 10, 2023 · 4 comments · Fixed by #1893
Labels

Comments

@jefftrull
Copy link
Contributor

jefftrull commented Feb 10, 2023

Operating System

Linux

Board

n/a

Firmware

examples/device/cdc_msc

What happened ?

Compilation under sdcc (4.0 and 4.2) fails on the assert at line 410 of cdc.h which checks the size of cdc_line_control_state_t. The exact layout of bitfields is apparently implementation-defined and sdcc makes a different choice than gcc in this case. See the SDCC discussion for more info. So this seems to be a kind of C standard conformance issue.

There is a straightforward workaround which I will supply in a PR.

How to reproduce ?

Build tinyusb under sdcc for 8051, for example by using my branch. Change src/class/cdc/cdc.h to match master and observe this error message:

src/class/cdc/cdc.h:410: warning 215: static assertion failed: "size is not correct"

Debug Log as txt file

No response

Screenshots

No response

I have checked existing issues, dicussion and documentation

  • I confirm I have checked existing issues, dicussion and documentation.
@jefftrull
Copy link
Contributor Author

jefftrull commented Feb 10, 2023

sdcc also rejects bitfields larger than 16 bits, complaining about this code; the PR will cover that as well. The error is:

src/class/cdc/cdc.h:380: error 31: bit-field size too wide for type (max 16 bits)

@hathach
Copy link
Owner

hathach commented Feb 13, 2023

can you specify which board/mcu you are using, if possible please add an PR to include the mcu into board support, so that we could include the SDCC into ci build to fix any future changes.

@jefftrull
Copy link
Contributor Author

This will eventually be for the (obsolete) ALi M5623 in certain Canon scanners; I am in the process of porting to SDCC but am facing code size issues that require significant changes to the TinyUSB core so a PR is somewhat in the future.

@hathach
Copy link
Owner

hathach commented Feb 13, 2023

thank you for the info, indeed the code size has been increased quite a bit lately due to lots of added features. Maybe in the future we should have some option to trim down the code size e.g no dynamic configuration etc..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants