Closed
Bug 1041914
Opened 11 years ago
Closed 11 years ago
Convert the fourth quarter of MFBT to Gecko style
Categories
(Core :: MFBT, defect)
Core
MFBT
Tracking
()
RESOLVED
FIXED
mozilla34
People
(Reporter: n.nethercote, Assigned: n.nethercote)
References
Details
Attachments
(2 files)
|
26.89 KB,
patch
|
Ms2ger
:
review+
|
Details | Diff | Splinter Review |
|
210.04 KB,
patch
|
Ms2ger
:
review+
|
Details | Diff | Splinter Review |
| Assignee | ||
Comment 1•11 years ago
|
||
As before: indentation changes in the first patch (|hg qdiff -w| is empty),
everything else in the second, and I'll merge before landing.
Attachment #8460041 -
Flags: review?(Ms2ger)
| Assignee | ||
Comment 2•11 years ago
|
||
Attachment #8460042 -
Flags: review?(Ms2ger)
Updated•11 years ago
|
Attachment #8460041 -
Flags: review?(Ms2ger) → review+
Comment 3•11 years ago
|
||
Comment on attachment 8460042 [details] [diff] [review]
...the rest..
Review of attachment 8460042 [details] [diff] [review]:
-----------------------------------------------------------------
::: mfbt/STYLE
@@ +1,1 @@
> +MFBT uses standard Mozilla style, with the following exceptions.
Some parts of this file might be useful to keep.
::: mfbt/tests/TestAtomics.cpp
@@ +14,5 @@
> using mozilla::Relaxed;
> using mozilla::ReleaseAcquire;
> using mozilla::SequentiallyConsistent;
>
> +#define A(a,b) MOZ_RELEASE_ASSERT(a,b)
Undef it at the end
::: mfbt/tests/TestBinarySearch.cpp
@@ +19,5 @@
> };
>
> struct GetAge
> {
> + Vector<Person> &mV;
& to the left
@@ +20,5 @@
>
> struct GetAge
> {
> + Vector<Person> &mV;
> + explicit GetAge(Vector<Person> &aV) : mV(aV) {}
Ditto
::: mfbt/tests/TestEndian.cpp
@@ +29,5 @@
> }
>
> template<typename T>
> void
> +TestSingleNoSwap(T aVal, T aUnswappedVal)
I prefer spelling out "value"
::: mfbt/tests/TestFloatingPoint.cpp
@@ +24,5 @@
> using mozilla::PositiveInfinity;
> using mozilla::SpecificNaN;
> using mozilla::UnspecifiedNaN;
>
> +#define A(a) MOZ_RELEASE_ASSERT(a)
undef
::: mfbt/tests/TestPoisonArea.cpp
@@ +227,2 @@
> {
> + if (aPage >= (uintptr_t)sInfo_.lpMaximumApplicationAddress &&
return aPage >= ...
@@ +273,2 @@
> {
> + if (madvise(reinterpret_cast<void*>(aPage), PageSize(), MADV_NORMAL)) {
return madvise(...
@@ +300,5 @@
> return result;
> + }
> +
> + // First see if we can allocate the preferred poison address from the OS.
> + uintptr_t candidate = (0xF0DEAFFF & ~(PageSize()-1));
Spaces around -
@@ +302,5 @@
> +
> + // First see if we can allocate the preferred poison address from the OS.
> + uintptr_t candidate = (0xF0DEAFFF & ~(PageSize()-1));
> + void* result = ReserveRegion(candidate, false);
> + if (result == (void*)candidate) {
reinterpret_cast
@@ +326,5 @@
> + // consolation prize?
> + if (result != MAP_FAILED) {
> + printf("INFO | poison area allocated at 0x%.*" PRIxPTR
> + " (consolation prize)\n", SIZxPTR, (uintptr_t)result);
> + return (uintptr_t)result;
reinterpret_cast
@@ +335,5 @@
> + result = ReserveRegion(0, false);
> + if (result != MAP_FAILED) {
> + printf("INFO | poison area allocated at 0x%.*" PRIxPTR
> + " (fallback)\n", SIZxPTR, (uintptr_t)result);
> + return (uintptr_t)result;
reinterpret_cast
@@ +356,5 @@
> return 0;
> }
> printf("INFO | positive control allocated at 0x%.*" PRIxPTR "\n",
> SIZxPTR, (uintptr_t)result);
> return (uintptr_t)result;
reinterpret_cast
@@ +373,5 @@
> }
>
> // Fill the page with return instructions.
> + RETURN_INSTR_TYPE* p = (RETURN_INSTR_TYPE*)result;
> + RETURN_INSTR_TYPE* limit = (RETURN_INSTR_TYPE*)(((char*)result) + PageSize());
reinterpret_cast
Attachment #8460042 -
Flags: review?(Ms2ger) → review+
| Assignee | ||
Comment 4•11 years ago
|
||
> ::: mfbt/STYLE
> @@ +1,1 @@
> > +MFBT uses standard Mozilla style, with the following exceptions.
>
> Some parts of this file might be useful to keep.
Why? And which parts? I don't know how to act on this suggestion.
> @@ +273,2 @@
> > {
> > + if (madvise(reinterpret_cast<void*>(aPage), PageSize(), MADV_NORMAL)) {
>
> return madvise(...
I did |return !!madvise(...);|, because madvise returns 0 or -1.
| Assignee | ||
Comment 5•11 years ago
|
||
Comment 6•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
Updated•11 years ago
|
QA Whiteboard: [qa-]
You need to log in
before you can comment on or make changes to this bug.
Description
•