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

[meta] Use memcpy instead of cast to prevent strict-aliasing error #723

Merged
merged 1 commit into from
Dec 1, 2020

Conversation

kcudnik
Copy link
Collaborator

@kcudnik kcudnik commented Dec 1, 2020

No description provided.

@kcudnik
Copy link
Collaborator Author

kcudnik commented Dec 1, 2020

hope will fix sonic-net/sonic-buildimage#6064

18:28:15  MetaKeyHasher.cpp: In function 'std::size_t sai_get_hash(const sai_fdb_entry_t&)':
18:28:15  MetaKeyHasher.cpp:168:49: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
18:28:15       return *(const uint32_t*)(&fe.mac_address[2]);
18:28:15                                                   ^

@kcudnik
Copy link
Collaborator Author

kcudnik commented Dec 1, 2020

retest vs please

@kcudnik kcudnik requested a review from daall December 1, 2020 20:16
@@ -160,12 +160,20 @@ static inline std::size_t sai_get_hash(
return ne.ip_address.addr_family;
}

static_assert(sizeof(uint32_t) == 4, "uint32_t expected to be 4 bytes");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

static_assert [](start = 0, length = 13)

This assertion seems not needed. Do you anticipate some failure case?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a surprise for some wired platforms :D

@kcudnik kcudnik merged commit d814d2c into sonic-net:master Dec 1, 2020
@kcudnik kcudnik deleted the castfix branch December 1, 2020 23:13
pettershao-ragilenetworks pushed a commit to pettershao-ragilenetworks/sonic-sairedis that referenced this pull request Nov 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants