-
Notifications
You must be signed in to change notification settings - Fork 268
Fix compiler warning in htons()
#161
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
Conversation
Memory usage change @ 68ac2ae
Click for full report table
Click for full report CSV
|
htons()
and htonl()
. Save 32 bytes of memoryhtons()
and htonl()
. Save at least 32 bytes of memory
@aentinger @cmaglie @per1234 I see from the commit history you were the last to commit to this repo. Would a review be possible? I'm not sure who to ask. |
and you say you tested DNS and DHCP with this? because it can't work with the numbers in host order of Arduinos. the macros swap the endianness between host and network order.
output
|
@JAndrassy Very good point. Thank you. I think it's best for me to focus on fixing the compiler warning 😅 . I reverted most of the changes but added comments above the macros. |
htons()
and htonl()
. Save at least 32 bytes of memoryhtons()
Memory usage change @ 5509403
Click for full report table
Click for full report CSV
|
any updates on this? |
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 👍
Btw is this still an issue in the merged changes? |
it is not. @gudnimg removed from this PR all problematic changes except of a super simple change to avoid the warning. |
Fixes #151
Fixes #122
The macros are completely redundant. Using for examplehtons()
on auint16_t
just wastes cycles if the compiler doesn't optimise it out.htons()
converts a variable to 16 bits (which does nothing for a 16 bit variable). Same goes forhtonl()
for 32-bit variables.Building these changes in a project of mine shows the memory footprint is reduced by 32 bytes. This will also fix two issues related to overflow compiler warnings withhtons()
.Edit: This PR will only fix the compiler warning in the linked issues.