calling InitRuntimeNumberState when instatiating the runtime

RESOLVED FIXED

Status

()

Core
JavaScript Engine
--
enhancement
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Igor Bukanov, Assigned: Igor Bukanov)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
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.
(Assignee)

Comment 1

6 years ago
Created attachment 539768 [details] [diff] [review]
v1

In addition to the number state initialization the patch also moves script runtime setup to JSRuntime:init.
Attachment #539768 - Flags: review?(anygregor)
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.
Attachment #539768 - Flags: review?(anygregor) → review+
(Assignee)

Comment 3

6 years ago
http://hg.mozilla.org/tracemonkey/rev/cd2baff53251 - pushed with an extra comments
Whiteboard: fixed-in-tracemonkey
cdleary-bot mozilla-central merge info:
http://hg.mozilla.org/mozilla-central/rev/cd2baff53251
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.