Closed
Bug 1071998
Opened 10 years ago
Closed 10 years ago
Use js_free to free JSShelContextData in the JS shell.
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla35
People
(Reporter: ggp, Assigned: ggp)
Details
Attachments
(1 file, 1 obsolete file)
1013 bytes,
patch
|
ggp
:
review+
|
Details | Diff | Splinter Review |
Since it's allocated with js_calloc, we should be releasing it with js_free.
Assignee | ||
Comment 1•10 years ago
|
||
Attachment #8494109 -
Flags: review?(jorendorff)
Comment 2•10 years ago
|
||
Utility.h says:
> static inline void js_free(void* p)
> {
> free(p);
> }
I guess I don't mind landing this, but don't get the wrong impression: there are many more mismatched uses of js_malloc with free or malloc with js_free. It's probably not worth your time to track them all down (but be my guest)!
Updated•10 years ago
|
Attachment #8494109 -
Flags: review?(jorendorff) → review+
Assignee | ||
Comment 3•10 years ago
|
||
Thanks for the review! Just to give you some context on why I proposed this: I'm experimenting with replacing SpiderMonkey's memory allocator, which is supported to some extent by Utility.h via the JS_USE_CUSTOM_ALLOCATOR macro. As such, my |js_free| doesn't really delegate to |free|, but rather gives memory back to a separate heap -- and every |free| to memory allocated through |js_malloc| causes a crash in my builds. Like I said, this is just experimentation (I plan on moving this down to libmemory if it gets anywhere), but these mismatches are not too hard to track down, and I can't proceed without fixing the one I come across, so I figured I might as well patch them :)
Assignee | ||
Comment 4•10 years ago
|
||
Updating the patch to fix the typo in the commit message, carrying over r+. Try: https://tbpl.mozilla.org/?tree=Try&rev=b14fe43da44f
Attachment #8494109 -
Attachment is obsolete: true
Attachment #8495191 -
Flags: review+
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → ggoncalves
Comment 5•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/ca1f93667348
Keywords: checkin-needed
Comment 6•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/ca1f93667348
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in
before you can comment on or make changes to this bug.
Description
•