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)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla35

People

(Reporter: ggp, Assigned: ggp)

Details

Attachments

(1 file, 1 obsolete file)

Since it's allocated with js_calloc, we should be releasing it with js_free.
Attachment #8494109 - Flags: review?(jorendorff)
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)!
Attachment #8494109 - Flags: review?(jorendorff) → review+
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 :)
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+
Keywords: checkin-needed
Assignee: nobody → ggoncalves
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.

Attachment

General

Created:
Updated:
Size: