-
Notifications
You must be signed in to change notification settings - Fork 746
Zero out padding in custom Default trait implementations #2051
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! This looks good. There are some libclang-version-specific tests that need updating (CI is red because of that), but with the bit below addressed and those updated this looks great.
Previously, we were using `std::mem::zeroed()` which unfortunately does not necessarily zero out padding. It'd be better if the padding is zeroed out because some libraries are sensitive to non-zero'd out bytes, especially when forward/backward compatability is involved. This commit ensures all bytes are zeroed out in custom Default trait implementations.
Waiting for the CI to run again so I can just copy paste the diffs from other clang versions |
I think I fixed up the fixtures. Another CI run should confirm |
Bump for CI |
Getting close! Bumping again |
I really hate that github policy of having to explicitly approve CI runs. I did plan to fix this up and merge it whenever I got a chance, but if you finish it up that's even better of course, thanks! :) |
(I should also figure out a way to make runs not stop at the first failure...) |
If you want to merge as-is and fixup later, that'd work for me too. |
No, I meant pulling your changes, fix up as needed myself, and push with the fixes. I may be able to do it tomorrow if you don't get it green by then, otherwise later this week. |
Woohoo! Got all of them |
Sweet, thanks! |
Previously, we were using
std::mem::zeroed()
which unfortunately doesnot necessarily zero out padding. It'd be better if the padding is
zeroed out because some libraries are sensitive to non-zero'd out bytes,
especially when forward/backward compatability is involved.
This commit ensures all bytes are zeroed out in custom Default trait
implementations.
This closes #2050 .
Note that I kept these commits separate to help with review. They should
probably be squashed before merge so tests are still bisectable.