Closed Bug 283545 Opened 20 years ago Closed 19 years ago

OOM crash [@ nsFTPDirListingConv::OnDataAvailable]

Categories

(Core Graveyard :: Networking: FTP, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: timeless, Assigned: bastiaan)

References

()

Details

(Keywords: crash)

Crash Data

Attachments

(1 file, 5 obsolete files)

checking only some allocations isn't enough :)
Attached patch fix (obsolete) — Splinter Review
Assignee: dougt → b.jacques
Status: NEW → ASSIGNED
Attached patch better fix (obsolete) — Splinter Review
Attachment #187675 - Attachment is obsolete: true
Attachment #187784 - Flags: superreview?(darin)
Attachment #187784 - Flags: review?(cbiesinger)
Attached patch even better fix (obsolete) — Splinter Review
nsCString -> nsCAutoString
Attachment #187784 - Attachment is obsolete: true
Attachment #187787 - Flags: superreview?(darin)
Attachment #187787 - Flags: review?(cbiesinger)
Attachment #187784 - Flags: superreview?(darin)
Attachment #187784 - Flags: review?(cbiesinger)
why not use nsAutoArrayPtr<char> and new char[...]? That way, you can avoid the
goto, and just return.
OS: Windows XP → All
Hardware: PC → All
Attached patch address comment (obsolete) — Splinter Review
Good idea.
Attachment #187787 - Attachment is obsolete: true
Attachment #187787 - Flags: superreview?(darin)
Attachment #187787 - Flags: review?(cbiesinger)
Attachment #187799 - Flags: review?(cbiesinger)
Comment on attachment 187799 [details] [diff] [review]
address comment

	 buffer = ToNewCString(mBuffer);

oh... I didn't realize that buffer gets a new value... you are now mismatching
allocators
Attachment #187799 - Flags: review?(cbiesinger) → review-
Attached patch address comments (obsolete) — Splinter Review
Attachment #187799 - Attachment is obsolete: true
Attachment #187846 - Flags: review?(cbiesinger)
Comment on attachment 187846 [details] [diff] [review]
address comments

+	 nsAutoArrayPtr<char> tmpBuffer (new char[mBuffer.Length()+1]);

I can't figure out why you didn't just reuse buffer here... or does:
  buffer = new char[...];
not work?

I think I wouldn't have removed the blank line here:
     rv = NS_NewCStringInputStream(getter_AddRefs(inputData), indexFormat);
-    if (NS_FAILED(rv)) return rv;
-
Attachment #187846 - Flags: review?(cbiesinger) → review+
Attached patch address commentsSplinter Review
(In reply to comment #8)
> (From update of attachment 187846 [details] [diff] [review] [edit])
> +	 nsAutoArrayPtr<char> tmpBuffer (new char[mBuffer.Length()+1]);
> 
> I can't figure out why you didn't just reuse buffer here... or does:
>   buffer = new char[...];
> not work?
It does, but I assumed it didn't, because |nsAutoArrayPtr<char> buffer = new
char[..];| throws a compiler error. I've changed the code to do what you
described, though.

> I think I wouldn't have removed the blank line here:
>      rv = NS_NewCStringInputStream(getter_AddRefs(inputData), indexFormat);
> -    if (NS_FAILED(rv)) return rv;
> -
done.
Attachment #187846 - Attachment is obsolete: true
Attachment #187875 - Flags: superreview?(darin)
Attachment #187875 - Flags: superreview?(darin) → superreview+
Attachment #187875 - Flags: approval1.8b3?
Attachment #187875 - Flags: approval1.8b3? → approval1.8b3+
Checked in by timeless (2005-07-08 12:40).
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Crash Signature: [@ nsFTPDirListingConv::OnDataAvailable]
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: