Skip to content

Conversation

@bitcartel
Copy link
Contributor

@bitcartel bitcartel commented Apr 17, 2018

Backports the first commit from bitcoin/bitcoin#9512 to avoid unaligned access in crypto i/o.

@bitcartel bitcartel added C-cleanup Category: PRs that clean code up or issues documenting cleanup. C-audit Category: Issues and tasks related to audit findings LeastAuthority finding labels Apr 17, 2018
@bitcartel bitcartel added this to the v1.1.1 milestone Apr 17, 2018
@bitcartel bitcartel requested review from daira and str4d April 17, 2018 16:24
@bitcartel bitcartel changed the title Avoid unaligned access in crypto i/o Closes #1249 - Least Authority Issue C Apr 17, 2018
Copy link
Contributor

@str4d str4d left a comment

Choose a reason for hiding this comment

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

utACK. I checked that this matches the upstream commit.

@str4d
Copy link
Contributor

str4d commented Apr 19, 2018

@zkbot try

@zkbot
Copy link
Contributor

zkbot commented Apr 19, 2018

⌛ Trying commit 0b6594f with merge 10517df5c68479aed1923d52908aba35ef8d9d4f...

@zkbot
Copy link
Contributor

zkbot commented Apr 19, 2018

☀️ Test successful - pr-try
State: approved= try=True

Copy link
Contributor

@daira daira left a comment

Choose a reason for hiding this comment

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

This isn't my preferred way to do this kind of thing (I prefer explicitly loading bytes and shifting; a good compiler should be able to recognize that idiom and replace with the efficient code for the platform). But it's correct as written here (because the uint*_t types are guaranteed to be two's complement and have no padding bits), so utACK.

@daira
Copy link
Contributor

daira commented Apr 21, 2018

@zkbot r+

@zkbot
Copy link
Contributor

zkbot commented Apr 21, 2018

📌 Commit 0b6594f has been approved by daira

@zkbot
Copy link
Contributor

zkbot commented Apr 21, 2018

⌛ Testing commit 0b6594f with merge 0e65c24...

zkbot added a commit that referenced this pull request Apr 21, 2018
Closes #1249 - Least Authority Issue C

Backports the first commit from bitcoin/bitcoin#9512 to avoid unaligned access in crypto i/o.
@zkbot
Copy link
Contributor

zkbot commented Apr 21, 2018

☀️ Test successful - pr-merge
Approved by: daira
Pushing 0e65c24 to master...

@zkbot zkbot merged commit 0b6594f into zcash:master Apr 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-audit Category: Issues and tasks related to audit findings C-cleanup Category: PRs that clean code up or issues documenting cleanup. LeastAuthority finding

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants