Closed Bug 215768 Opened 22 years ago Closed 22 years ago

nsAutoPtr is used where nsAutoArrayPtr should've been used : nsUTF8ConverterService.cpp

Categories

(Core :: Internationalization, defect)

defect
Not set
critical

Tracking

()

RESOLVED FIXED

People

(Reporter: jshin1987, Assigned: jshin1987)

References

Details

(Keywords: memory-leak)

Attachments

(1 file)

While fixing bug 162765, I made a mistake of using nsAutoPtr instead of nsAutoArrayPtr in nsUTF8ConverterService.cpp I'm gonna upload a patch (2-liner) that also uses a better workaround for bug 210297 that I forgot to land.
I guess using nsAutoPtr (delete) in place of nsAutoArrayPtr (delete []) can lead to 'who knows what'. It's curious nothing has yet been reported.
Comment on attachment 129575 [details] [diff] [review] a patch (2-liner) asking for r/sr. it's a simple fix but is necessary to prevent a potential diaster.
Attachment #129575 - Flags: superreview?(dbaron)
Attachment #129575 - Flags: review?(bugmail)
Comment on attachment 129575 [details] [diff] [review] a patch (2-liner) sr=bzbarsky. Using delete instead of delete[] has no bad side effects that I know of apart from not calling destructors of things in the array (not an issue here) and possibly leaking most of the array (which would not be so cool, but would not obviously explode). I think we would really like this for 1.5b, though, since the memory leak aspect could be a little unfortunate.
Attachment #129575 - Flags: superreview?(dbaron)
Attachment #129575 - Flags: superreview+
Attachment #129575 - Flags: approval1.5b?
Severity: normal → critical
Keywords: mlk
I've never heard of using |delete[]| instead of |delete| causing partial leaks. (It could cause other problems if the system is using different allocators for |new| and |new[]|, which is theoretically possible, although unlikely.)
Comment on attachment 129575 [details] [diff] [review] a patch (2-liner) a=asa (on behalf of drivers) for checkin to Mozilla 1.5beta.
Attachment #129575 - Flags: approval1.5b? → approval1.5b+
patch checked in. thanks all
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
See Also: → 1241901
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: