M18a2 topcrash in safefileoutputstream [@ nsSafeFileOutputStream::Init]

VERIFIED FIXED

Status

()

Core
Networking: File
--
critical
VERIFIED FIXED
14 years ago
14 years ago

People

(Reporter: Michiel van Leeuwen (email: mvl+moz@), Assigned: dwitte@gmail.com)

Tracking

({crash, fixed-aviary1.0, topcrash})

Trunk
x86
Windows XP
crash, fixed-aviary1.0, topcrash
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(crash signature)

Attachments

(1 attachment, 2 obsolete attachments)

2.50 KB, patch
Michiel van Leeuwen (email: mvl+moz@)
: review+
Darin Fisher
: superreview+
Details | Diff | Splinter Review
Severity: normal → critical
(Assignee)

Comment 1

14 years ago
Created attachment 153828 [details] [diff] [review]
nullcheck file

check in the stream init, to make it consistent with nsFileOutputStream.
(Assignee)

Comment 2

14 years ago
Created attachment 153829 [details] [diff] [review]
nullcheck mCookieFile too

cookies is called without a profile at startup, in seamonkey anyway (not sure
about fx). we initialize it for some silly thing before the profile manager
comes up. i can't remember why, but i hate it. i don't know why we haven't seen
this crash ourselves.
Assignee: darin → dwitte
Status: NEW → ASSIGNED
(Assignee)

Updated

14 years ago
Attachment #153828 - Flags: superreview?(darin)
Attachment #153828 - Flags: review?(mvl)
(Assignee)

Updated

14 years ago
Attachment #153829 - Flags: superreview?(darin)
Attachment #153829 - Flags: review?(mvl)
(Reporter)

Comment 3

14 years ago
Comment on attachment 153828 [details] [diff] [review]
nullcheck file

>-        NS_ENSURE_STATE(mTargetFile);
Hmm, maybe you should leave that check there. It never hurts to be sure. A bad
consumer can call Finish() even when Init() failed.

Other checks is good. r=mvl
Attachment #153828 - Flags: review?(mvl) → review+
(Reporter)

Comment 4

14 years ago
Comment on attachment 153829 [details] [diff] [review]
nullcheck mCookieFile too

>+  NS_ENSURE_ARG(mCookieFile);
NS_ENSURE_STATE might be more appropiate. It isn't an arg.
Also, if you know that it will warn on startup because cookies start when there
is no profile, is it worth warning?
(Assignee)

Comment 5

14 years ago
you're right, it's not. i'll make it an |if| check instead.

Comment 6

14 years ago
please post one final patch for this bug, thanks!

Comment 7

14 years ago
Comment on attachment 153828 [details] [diff] [review]
nullcheck file

i agree with mvl, keep the NS_ENSURE_STATE(mTargetFile)

shouldn't mTargetFileExists be initialized in the ctor?
Attachment #153828 - Flags: superreview?(darin) → superreview-

Comment 8

14 years ago
Comment on attachment 153829 [details] [diff] [review]
nullcheck mCookieFile too

minus in favor of NS_ENSURE_STATE as suggested by mvl.
Attachment #153829 - Flags: superreview?(darin) → superreview-

Comment 9

14 years ago
Adding topcrash info for tracking, this is a topcrasher with Mozilla 1.8 alpha2.
Keywords: topcrash
Summary: topcrash in safefileoutputstream [@ nsSafeFileOutputStream::Init] → M18a2 topcrash in safefileoutputstream [@ nsSafeFileOutputStream::Init]
(Assignee)

Updated

14 years ago
Attachment #153829 - Flags: review?(mvl)
(Assignee)

Comment 10

14 years ago
Created attachment 153931 [details] [diff] [review]
here we go
Attachment #153828 - Attachment is obsolete: true
Attachment #153829 - Attachment is obsolete: true
(Assignee)

Updated

14 years ago
Attachment #153931 - Flags: superreview?(darin)

Comment 11

14 years ago
Comment on attachment 153931 [details] [diff] [review]
here we go

sr=darin
Attachment #153931 - Flags: superreview?(darin) → superreview+
(Assignee)

Updated

14 years ago
Attachment #153931 - Flags: review?(mvl)
(Reporter)

Updated

14 years ago
Attachment #153931 - Flags: review?(mvl) → review+
(Assignee)

Updated

14 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
(Assignee)

Updated

14 years ago
Whiteboard: needed-aviary1.0
(Assignee)

Updated

14 years ago
Keywords: fixed-aviary1.0
Whiteboard: needed-aviary1.0

Comment 12

14 years ago
V/fixed, per lxr.
Status: RESOLVED → VERIFIED
Crash Signature: [@ nsSafeFileOutputStream::Init]
You need to log in before you can comment on or make changes to this bug.