936710 - Add an option to propagate flags to nested shells

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: propagate-flags.patch

Generic enough for future purpose, useful enough for propagating --no-sse4 in bug 935791.

Comment 1

[Jason Orendorff [:jorendorff]]

strdup() is malloc, so this leaks memory, right?

Comment 2

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: propagate-flags.patch

Doh, thanks Jason! I am so used to the realm of the JITs, where the |new| operators are overloaded to use temporary arenas, that I forgot about basic memory management in the shell. Shame on me...

As I understand it, even if there is *still* a use of strdup here, it's not a leak as the AutoCStringVector deallocates strings in its destructor. Moreover, it is the way it has been done for the other arguments that are passed to the nested shell.

Comment 3

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: propagate-flags.patch

Thanks! Filled the r= field, carrying forward the r+ (still can't push directly...)

Comment 4

[Chris Peterson [:cpeterson]]

Benjamin: your patch never calls PropagateFlagToNestedShells() and nevers writes to sPropagatedFlags. Will a subsequent patch use that code? Also, should sPropagatedFlags be static, not global?

Comment 5

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: propagate-flags.patch

Indeed, it's better for it to be a static variable. This feature is motivated by bug 935791 which will use it.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6c071c069b22

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/6c071c069b22