Closed
Bug 351739
Opened 18 years ago
Closed 18 years ago
Memory leak in |JS_dtobasestr| (jsdtoa.c)
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
People
(Reporter: kherron+mozilla, Assigned: sciguyryan)
References
(Blocks 1 open bug, )
Details
(Keywords: coverity, fixed1.8.0.10, fixed1.8.1.2)
Attachments
(1 file, 3 obsolete files)
|
(deleted),
patch
|
igor
:
review+
jay
:
approval1.8.1.2+
jay
:
approval1.8.0.10+
|
Details | Diff | Splinter Review |
This is coverity ID 68. Please see the sample URL. The allocation at line 2928 is leaked if the call to |d2b| or |lshift| return null.
|
Reporter | |
Updated•18 years ago
|
|
||
Comment 1•18 years ago
|
||
Check NSPR's prdtoa.c. The JS version forked long ago from David M. Gay's netlib hosted dtoa.c, and I think NSPR refreshed more recently.
/be
|
||
Comment 2•18 years ago
|
||
NSPR's prdtoa.c doesn't have the equivalent of the
JS_dtobasestr function.
|
Assignee | |
Comment 3•18 years ago
|
||
Patch v1.
Simply use |free(buffer)| before the function returns.
Assignee: general → sciguyryan+bugzilla
Status: NEW → ASSIGNED
Attachment #242217 -
Flags: superreview?(brendan)
Attachment #242217 -
Flags: review?(brendan)
|
||
Comment 4•18 years ago
|
||
Same leak around line 3007-ish?
|
|
Assignee | |
Comment 5•18 years ago
|
||
(In reply to comment #4)
> Same leak around line 3007-ish?
>
Certainly looks like it. |buffer| isn't freed before then so I guess your right. I'll update the patch! Thanks for pointing that out :)
|
|
Assignee | |
Comment 6•18 years ago
|
||
Updated to add a free for the same leak spotted by Brian Crowder.
Attachment #242217 -
Attachment is obsolete: true
Attachment #242222 -
Flags: superreview?(brendan)
Attachment #242222 -
Flags: review?(brendan)
Attachment #242217 -
Flags: superreview?(brendan)
Attachment #242217 -
Flags: review?(brendan)
|
|
Assignee | |
Updated•18 years ago
|
Attachment #242222 -
Flags: superreview?(brendan)
Attachment #242222 -
Flags: review?(igor)
Attachment #242222 -
Flags: review?(brendan)
|
||
Comment 7•18 years ago
|
||
Comment on attachment 242222 [details] [diff] [review]
Patch v1.1
This is not a pacth against the trunk. Please update it to reflect bug 357392.
Attachment #242222 -
Flags: review?(igor)
|
|
Assignee | |
Comment 8•18 years ago
|
||
Patch v2.0
* Updated to latest trunk build.
Attachment #242222 -
Attachment is obsolete: true
Attachment #251760 -
Flags: review?(igor)
|
|
||
Comment 9•18 years ago
|
||
Comment on attachment 251760 [details] [diff] [review]
Patch v2.0
I will review=+ for the patch with changed order of free and RELEASE_DTOA_LOCK calls to follow LIFO pattern.
|
|
Assignee | |
Comment 10•18 years ago
|
||
Patch v2.1
* Same as patch v2.0 but updated to follow the LIFO pattern as suggested by Igor Bukanov.
Attachment #251760 -
Attachment is obsolete: true
Attachment #251764 -
Flags: review?(igor)
Attachment #251760 -
Flags: review?(igor)
|
|
||
Updated•18 years ago
|
Attachment #251764 -
Flags: review?(igor) → review+
|
|
Assignee | |
Updated•18 years ago
|
Whiteboard: [checkin needed]
|
|
||
Comment 11•18 years ago
|
||
I committed the patch from comment 10 to the trunk:
Checking in jsdtoa.c;
/cvsroot/mozilla/js/src/jsdtoa.c,v <-- jsdtoa.c
new revision: 3.39; previous revision: 3.38
done
Please mark the bug fixed after verifying that the committed code is the right one.
Flags: blocking1.8.1.2?
Flags: blocking1.8.0.10?
|
|
||
Updated•18 years ago
|
Attachment #251764 -
Flags: approval1.8.1.2?
Attachment #251764 -
Flags: approval1.8.0.10?
|
||
Updated•18 years ago
|
Whiteboard: [checkin needed]
|
||
Comment 12•18 years ago
|
||
Comment on attachment 251764 [details] [diff] [review]
Patch v2.1
Approved for both branches, a=jay for drivers.
Attachment #251764 -
Flags: approval1.8.1.2?
Attachment #251764 -
Flags: approval1.8.1.2+
Attachment #251764 -
Flags: approval1.8.0.10?
Attachment #251764 -
Flags: approval1.8.0.10+
|
|
||
Updated•18 years ago
|
Flags: blocking1.8.1.2?
Flags: blocking1.8.1.2+
Flags: blocking1.8.0.10?
Flags: blocking1.8.0.10+
|
|
||
Comment 13•18 years ago
|
||
I committed the patch from comment 10 to MOZILLA_1_8_0_BRANCH:
Checking in jsdtoa.c;
/cvsroot/mozilla/js/src/jsdtoa.c,v <-- jsdtoa.c
new revision: 3.33.10.3; previous revision: 3.33.10.2
done
|
|
||
Comment 14•18 years ago
|
||
I also committed yesterday the patch from comment 10 to MOZILLA_1_8_BRANCH but forgot to update the bug. The committed version was 3.33.2.4.
Keywords: fixed1.8.1.2
|
||
Updated•18 years ago
|
Flags: in-testsuite-
|
||
Updated•6 years ago
|
Blocks: coverity-analysis
You need to log in
before you can comment on or make changes to this bug.
Description
•