Closed
Bug 1199624
Opened 9 years ago
Closed 9 years ago
Add missing cstring includes to security/pkix/lib/.
Categories
(Core :: Security, defect)
Tracking
()
RESOLVED
FIXED
mozilla43
Tracking | Status | |
---|---|---|
firefox43 | --- | fixed |
People
(Reporter: jacek, Assigned: jacek)
References
Details
Attachments
(1 file, 2 obsolete files)
2.17 KB,
patch
|
briansmith
:
review+
|
Details | Diff | Splinter Review |
Those includes are needed on mingw for memcmp/memmove/memset functions since string include was removed from Char16.h.
Attachment #8654039 -
Flags: review?(brian)
Comment 1•9 years ago
|
||
Comment on attachment 8654039 [details] [diff] [review]
fix
Review of attachment 8654039 [details] [diff] [review]:
-----------------------------------------------------------------
Thanks for the patch.
I believe the patch for bug 1189891 broke this.
The proper fix needs to be consistent with the fix for bug 1189891: change the code to use std::equals and similar instead of mem*, and remove all #include <cstring>.
Attachment #8654039 -
Flags: review?(brian) → review-
Assignee | ||
Comment 2•9 years ago
|
||
Thanks for the review. The attached patch avoids dependency on cstring.
Attachment #8654039 -
Attachment is obsolete: true
Attachment #8654502 -
Flags: review?(brian)
Comment 3•9 years ago
|
||
Comment on attachment 8654502 [details] [diff] [review]
fix
Review of attachment 8654502 [details] [diff] [review]:
-----------------------------------------------------------------
Thanks! Just one small issue.
::: security/pkix/lib/pkixnames.cpp
@@ +1700,5 @@
> address + (2u * static_cast<size_t>(contractionIndex)),
> componentsToMove * 2u);
> // Fill in the contracted area with zeros.
> + uint8_t* ptr = address + 2u * static_cast<size_t>(contractionIndex);
> + std::fill(ptr, ptr + (8u - static_cast<size_t>(numComponents)) * 2u, 0u);
Please use std::fill_n here, without |ptr|.
Attachment #8654502 -
Flags: review?(brian) → review-
Assignee | ||
Comment 4•9 years ago
|
||
Thanks. The attached version uses fill_n.
Attachment #8654502 -
Attachment is obsolete: true
Attachment #8654515 -
Flags: review?(brian)
Updated•9 years ago
|
Attachment #8654515 -
Flags: review?(brian) → review+
Updated•9 years ago
|
Keywords: checkin-needed
Keywords: checkin-needed
Comment 6•9 years ago
|
||
Comment 7•9 years ago
|
||
Static analysis bustage too.
https://treeherder.mozilla.org/logviewer.html#?job_id=13475944&repo=mozilla-inbound
Assignee | ||
Comment 8•9 years ago
|
||
There was a typo, I pushed fixed version to try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=78c436350e59
Sorry about that.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
You need to log in
before you can comment on or make changes to this bug.
Description
•