Closed Bug 803692 Opened 7 years ago Closed 7 years ago

Make SHA1Sum::update() take a void* instead of a uint8_t*

Categories

(Core :: MFBT, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla19
blocking-basecamp +
Tracking Status
firefox18 --- fixed

People

(Reporter: justin.lebar+bug, Assigned: justin.lebar+bug)

References

Details

Attachments

(1 file)

SHA1Sum::update() shouldn't care what type its data is; it's like memcpy, which takes a const void*.

Patch in a moment.
Blocks: 803684
Attached patch Patch, v1Splinter Review
Attachment #673410 - Flags: review?(jwalden+bmo)
Assignee: nobody → justin.lebar+bug
[basecamp-blocking request]
Follow the dependency chain back to a blocking-basecamp+ bug.
blocking-basecamp: --- → ?
Minusing this for the same reason that bug 803684 was minused. That said, this patch seems safe enough that I'd approve it for aurora.
This is structurally really broken.

I can't aurora? this patch until Jeff reviews it.  Aurora approval takes multiple days.  So by blocking- this bug, we're putting increasingly more pieces of approval and waiting in my critical path.

I just want to fix my bugs, but I'm spending half my time arguing about what's the right way to ask for approval and waiting for multiple serialized steps in the landing process.  It's an incredible waste of time.
Maybe I should just shove all relevant changes into one bug.  Then I can get blocking+ on that one bug and not have to go through this hassle.

That's the wrong thing to do, of course, but if we can't have a sane way to request approval to land, that seems like the best option I have.
Comment on attachment 673410 [details] [diff] [review]
Patch, v1

Review of attachment 673410 [details] [diff] [review]:
-----------------------------------------------------------------

::: mfbt/SHA1.cpp
@@ +128,5 @@
>    }
>  
>    while (len >= 64U) {
>      len -= 64U;
> +    shaCompress(&H[H2X], reinterpret_cast<const uint32_t*>(data));

Hmm.  Can someone pass in an unaligned address, such that this cast/read from it will be unaligned?  Looks like yet.  Followup to deal with this?
Attachment #673410 - Flags: review?(jwalden+bmo) → review+
> Hmm.  Can someone pass in an unaligned address, such that this cast/read from it will be unaligned?

Indeed; this could happen if someone hashes a uint8_t* off the stack, for example.  Good call.
Comment on attachment 673410 [details] [diff] [review]
Patch, v1

[Approval Request Comment]
No functional change; just changing a method signature.  Needed for dependent bug which is a basecamp+ blocker if you go back far enough.
Attachment #673410 - Flags: approval-mozilla-aurora?
Blocks: 781627
> Indeed; this could happen if someone hashes a uint8_t* off the stack, for example.  Good call.

Filed bug 804272.
https://hg.mozilla.org/mozilla-central/rev/d1cef2045028
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
blocking-basecamp: ? → +
Attachment #673410 - Flags: approval-mozilla-aurora?
You need to log in before you can comment on or make changes to this bug.