Last Comment Bug 664677 - calling InitRuntimeNumberState when instatiating the runtime
: calling InitRuntimeNumberState when instatiating the runtime
Status: RESOLVED FIXED
fixed-in-tracemonkey
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- enhancement (vote)
: ---
Assigned To: Igor Bukanov
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-06-16 02:16 PDT by Igor Bukanov
Modified: 2011-06-20 17:13 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v1 (19.44 KB, patch)
2011-06-16 05:11 PDT, Igor Bukanov
anygregor: review+
Details | Diff | Splinter Review

Description Igor Bukanov 2011-06-16 02:16:06 PDT
Currently js_InitRuntimeNumberState is called when constructing the first context. However, the implementation does not depend on JSContext *cx. As such it should be called when we create an instance of JSRuntime.
Comment 1 Igor Bukanov 2011-06-16 05:11:01 PDT
Created attachment 539768 [details] [diff] [review]
v1

In addition to the number state initialization the patch also moves script runtime setup to JSRuntime:init.
Comment 2 Gregor Wagner [:gwagner] 2011-06-17 08:30:04 PDT
Comment on attachment 539768 [details] [diff] [review]
v1

> 
>-    return rt->thousandsSeparator && rt->decimalSeparator && rt->numGrouping;
>+    memcpy(storage, thousandsSeparator, thousandsSeparatorSize);
>+    rt->thousandsSeparator = storage;
>+    storage += thousandsSeparatorSize;
>+
>+    memcpy(storage, decimalPoint, decimalPointSize);
>+    rt->decimalSeparator = storage;
>+    storage += decimalPointSize;
>+
>+    memcpy(storage, grouping, groupingSize);
>+    rt->numGrouping = grouping;
>+    return true;

Please add a short comment here describing your one malloc for 3 strings.

> }
> 
> void
>-js_FinishRuntimeNumberState(JSContext *cx)
>+FinishRuntimeNumberState(JSRuntime *rt)
> {
>-    JSRuntime *rt = cx->runtime;
>+    Foreground::free_(const_cast<char *>(rt->thousandsSeparator));
>+}
> 
>-    cx->free_((void *) rt->thousandsSeparator);
>-    cx->free_((void *) rt->decimalSeparator);
>-    cx->free_((void *) rt->numGrouping);
>-    rt->thousandsSeparator = rt->decimalSeparator = rt->numGrouping = NULL;
>-}
>+} /* namespace js */
> 


Here as well, explain that one free is enough.
Comment 3 Igor Bukanov 2011-06-17 09:08:44 PDT
http://hg.mozilla.org/tracemonkey/rev/cd2baff53251 - pushed with an extra comments
Comment 4 Chris Leary [:cdleary] (not checking bugmail) 2011-06-20 17:13:25 PDT
cdleary-bot mozilla-central merge info:
http://hg.mozilla.org/mozilla-central/rev/cd2baff53251

Note You need to log in before you can comment on or make changes to this bug.