Skip to content

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