mongoose-os-libs / homekit-adk

Port of Apple's HomeKit ADK to Mongoose OS
Apache License 2.0
23 stars 5 forks source link

tlv8 int parsing fixes #10

Closed markirb closed 1 year ago

markirb commented 2 years ago

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 commented 2 years ago

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

markirb commented 2 years ago

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

markirb commented 2 years ago

I looked into it more I guess it is only in C++ see https://stackoverflow.com/questions/3784996/why-does-left-shift-operation-invoke-undefined-behaviour-when-the-left-side-oper

rojer commented 2 years ago

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 commented 2 years ago

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 commented 1 year ago

@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 commented 1 year ago

ok, fair enough, merging

d4rkmen commented 1 year ago

upstream approved 🤣 since 8 mo