nsICacheMetadata::visitMetaData blows up in debug mode if the visitor returns false

RESOLVED FIXED in mozilla12

Status

()

Core
Networking: Cache
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Gijs, Assigned: Gijs)

Tracking

Trunk
mozilla12
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
http://mxr.mozilla.org/mozilla-central/source/netwerk/cache/nsCacheMetaData.cpp?rev=e7854b4d29ba#181

181     NS_ABORT_IF_FALSE(data == limit, "Metadata corrupted");

But data != limit whenever the visitor returns false. The documentation doesn't specify when that should happen (just that it returns a boolean).

Firebug currently uses this code and returns false, which crashes debug builds. I don't really know what the original goal of the assert was, but from what I can tell it can be removed.
(Assignee)

Comment 1

5 years ago
Created attachment 585705 [details] [diff] [review]
Option 1: Remove the assert

One thing we could do is nuke the assert...
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Attachment #585705 - Flags: review?(jduell.mcbugs)
(Assignee)

Comment 2

5 years ago
Created attachment 585706 [details] [diff] [review]
Option 2: Return early

Or we could keep the assert and return early.

NB: this returns NS_OK even if the visitor code fails. Whether that's desirable is up to you guys...
Attachment #585706 - Flags: review?(jduell.mcbugs)
Comment on attachment 585705 [details] [diff] [review]
Option 1: Remove the assert

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

Thanks for the patches, Gijs!

Michal is a better reviewer for them.
Attachment #585705 - Flags: review?(jduell.mcbugs) → review?(michal.novotny)
Attachment #585706 - Flags: review?(jduell.mcbugs) → review?(michal.novotny)
Attachment #585705 - Flags: review?(michal.novotny)
Attachment #585706 - Flags: review?(michal.novotny) → review+
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
(Assignee)

Updated

5 years ago
Depends on: 720212

Updated

5 years ago
Attachment #585705 - Attachment is obsolete: true
(Assignee)

Comment 4

5 years ago
Pushed to inbound: http://hg.mozilla.org/integration/mozilla-inbound/rev/2c215e31b7d0
Keywords: checkin-needed
(Assignee)

Updated

5 years ago
Attachment #585706 - Flags: checkin+
https://hg.mozilla.org/mozilla-central/rev/2c215e31b7d0
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla12
You need to log in before you can comment on or make changes to this bug.