Closed
Bug 1041914
Opened 9 years ago
Closed 9 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 |
This finishes the style conversion begun in bug 1014377 and bug 1026319 and bug 1036789.
![]() |
Assignee | |
Comment 1•9 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•9 years ago
|
||
Attachment #8460042 -
Flags: review?(Ms2ger)
Updated•9 years ago
|
Attachment #8460041 -
Flags: review?(Ms2ger) → review+
Comment 3•9 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•9 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•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/1b7d7dcf71e2
Comment 6•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/1b7d7dcf71e2
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
QA Whiteboard: [qa-]
You need to log in
before you can comment on or make changes to this bug.
Description
•