Closed Bug 838349 Opened 8 years ago Closed 8 years ago

bogus use of MOZ_ASSERT

Categories

(Toolkit :: Places, defect)

defect
Not set
minor

Tracking

()

RESOLVED FIXED
mozilla21

People

(Reporter: mats, Assigned: mak)

Details

Attachments

(1 file)

# grep -r 'MOZ_ASSERT("' .
./toolkit/components/places/History.cpp:      MOZ_ASSERT("Do not call me!");
./toolkit/components/places/Database.cpp:      MOZ_ASSERT("Trying to set an unknown journal mode.");
./toolkit/components/places/nsAnnotationService.cpp:        MOZ_ASSERT("Unsupported annotation type");
Assignee: nobody → mak77
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla21
Comment on attachment 710452 [details] [diff] [review]
patch v1.0

>Bug 838349 - Fix bogus use of assertions in Places
[...]
>-      MOZ_NOT_REACHED("Fetching favicon information failed unexpectedly.");
>+      MOZ_ASSERT(false, "Fetching favicon information failed unexpectedly.");

Was this change accidental?  The MOZ_NOT_REACHED usage looks correct to me there.
(in fact, perhaps of the other fixed chunks would be better as MOZ_NOT_REACHED("")...? Or is MOZ_ASSERT(false,"") better than NOT_REACHED for some reason?)
MOZ_NOT_REACHED is different from NS_NOTREACHED, in the fact it tells the compiler to optimize that code path to be not reachable.  My original intent there was just to put a NS_NOTREACHED but I was confused by the naming... MOZ_ASSERT is better too.
see bug 820686
I see -- thanks!
Version: unspecified → Trunk
https://hg.mozilla.org/mozilla-central/rev/b45420e3fa09
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.