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

tlv8 int parsing fixes #10

Merged
merged 1 commit into from
Feb 6, 2023
Merged

tlv8 int parsing fixes #10

merged 1 commit into from
Feb 6, 2023

Conversation

markirb
Copy link
Contributor

@markirb markirb commented Jul 6, 2022

this fixes undefined c behaviour when paring uint64_t, int64_t
(e.g. 255 << 32 has insufficient space as it is done on (u)int32)

and also on int32_t (255 << 24 is undefined)
see e.g. https://stackoverflow.com/questions/11644362/are-the-results-of-bitwise-operations-on-signed-integers-defined

this fixes undefined c behaviour when paring uint64_t, int64_t
(e.g. 255 << 32 has insufficient space as it is done on (u)int32) and also on int32_t (255 << 24 is undefined)
see e.g. https://stackoverflow.com/questions/11644362/are-the-results-of-bitwise-operations-on-signed-integers-defined
@rojer
Copy link
Contributor

rojer commented Jul 6, 2022

hm, this should be reported upstream - https://github.com/Apple/HomeKitADK/
i'd like to see what they say

@markirb
Copy link
Contributor Author

markirb commented Jul 6, 2022

Yeah will do. This bugged me a lot until I found it…

@markirb
Copy link
Contributor Author

markirb commented Jul 6, 2022

@rojer
Copy link
Contributor

rojer commented Jul 6, 2022

wow, that's subtle. your PR looks good but let's give them some time to respond. if at all possible i'd rather it be merged upstream and we update our copy, to minimize divergence.

@markirb
Copy link
Contributor Author

markirb commented Sep 10, 2022

The issue has been sitting there for some time now, I don't think anyone at Apple is currently interested in HomeKit... probably all working on matter

@timoschilling
Copy link
Contributor

@rojer It looks like https://github.com/Apple/HomeKitADK/ is dead, PR is open since 3/4 year and no code changes since 1 1/2 years. So it would be cool to get it merged. Shelly HomeKit already using this change and it works nicely.

@rojer
Copy link
Contributor

rojer commented Feb 6, 2023

ok, fair enough, merging

@rojer rojer merged commit 66d9125 into mongoose-os-libs:master Feb 6, 2023
@d4rkmen
Copy link

d4rkmen commented Feb 26, 2023

upstream approved 🤣 since 8 mo

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

Successfully merging this pull request may close these issues.

4 participants