Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork41
Loader improvements#264
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
pillo79 left a comment• edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
LGTM, not sure about the need for endianness checks as it would require a very big effort to go over the whole project (code and tools) to make sure there is proper support for it.
EDIT: please add a short commit message that explains why this is useful.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
andreagilardoni commentedNov 14, 2025
@pillo79 I believe that the endianness commit has little to no cost in terms of code space (on little endian platforms) and in a distant future where we may try a big endian platform we have one place less to check to make it compatible. I don't see any other reasons except that this is a free improvement in platform independent code. |
3d1157a to75baff8Compare| sketch_hdr.len=sys_le32_to_cpu(sketch_hdr.len); | ||
| sketch_hdr.magic=sys_le16_to_cpu(sketch_hdr.magic); | ||
| if (sketch_hdr.ver!=0x1||sketch_hdr.magic!=0x2341) { |
leonardocavagnisNov 14, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
| if (sketch_hdr.ver!=0x1||sketch_hdr.magic!=0x2341) { | |
| if (sketch_hdr.ver!=SKETCH_VER||sketch_hdr.magic!=SKETCH_MAGIC) { |
Consider replacing the magic numbers with#define constants for maintainability
leonardocavagnis left a comment
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.
LGTM :)
minor change suggested
This PR aims to:
byteorderzephyr librarystruct sketch_header_v1to a union which can simplify the procedure of converting a 16 byte buffer into a C struct and respect the strict aliasing rule.