-
-
Notifications
You must be signed in to change notification settings - Fork 37
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 our terms of service and privacy 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?
Loader improvements #264
Conversation
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.
|
@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 to
75baff8
Compare
| 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) { |
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.