Skip to content

IPAddress: allow misaligned source in constructor #5421

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

Merged
merged 2 commits into from
Dec 3, 2018

Conversation

d-a-v
Copy link
Collaborator

@d-a-v d-a-v commented Dec 3, 2018

fix #5420

@d-a-v d-a-v merged commit 31bee50 into esp8266:master Dec 3, 2018
@d-a-v d-a-v deleted the ipaddress branch December 3, 2018 15:59
@TD-er
Copy link
Contributor

TD-er commented Dec 8, 2018

Just to get an idea of the impact of this fix.
In what circumstances would this go wrong?

I have looked at my project's source code and it seems we're using this constructor of IPAddress from our settings file and maybe some other instances too.
This settings file is a struct which is read as a byte stream from the SPIFFS.
So it may differ per build where this struct is being allocated (on the sys stack).

In what circumstances is this going wrong? Is it compiler dependent?

@d-a-v
Copy link
Collaborator Author

d-a-v commented Dec 8, 2018

In what circumstances would this go wrong?

Before this PR and after the IPv6 commit, we were doing in #5420 uint32_t addr = *(uint32_t*)(4*x + 2) <- reading 4bytes from adresses not multiple of 4 causes an exception.

I have looked at my project's source code and it seems we're using this constructor of IPAddress from our settings file and maybe some other instances too.

This constructor now works like the previous one (before the IPv6 commit). Reading pointer byte by byte is misalignment-proof. memcpy() would work too.

@TD-er
Copy link
Contributor

TD-er commented Dec 8, 2018

OK, thanks for the explanation.

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.

Constructing IPAddress crashes ESP. Since 2.5.0. Works in 2.4.2.
2 participants