Bug 176722 – Make jsc dump the command line if JSC_dumpOption environment variable is set with a non-zero value.

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Mark Lam]

Currently, specifying the JSC_dumpOption=<non zero value> environment variable will cause JSC options to be dumped via dataLog.  This is useful for debugging what options are used for any specific test run.  This patch adds the dumping of the command line as well so that it's easier to determine the command line that test scripts invoked jsc with for any given test run (without needing to decode the layers of test scripts).

[Mark Lam]

Created attachment 320457 [details]
proposed patch.

[JF Bastien]

Comment on attachment 320457 [details]
proposed patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=320457&action=review

> Source/JavaScriptCore/jsc.cpp:3846
> +    if (dumpOptionsOption && strcmp(dumpOptionsOption, "0")) {

OptionsOptionOptionsOptionsOptionOptions

Code seems to usually do:
!strcasecmp(string, "none") || !strcasecmp(string, "no") || !strcasecmp(string, "false") || !strcmp(string, "0")

?

[Saam Barati]

Comment on attachment 320457 [details]
proposed patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=320457&action=review

>> Source/JavaScriptCore/jsc.cpp:3846
>> +    if (dumpOptionsOption && strcmp(dumpOptionsOption, "0")) {
> 
> OptionsOptionOptionsOptionsOptionOptions
> 
> Code seems to usually do:
> !strcasecmp(string, "none") || !strcasecmp(string, "no") || !strcasecmp(string, "false") || !strcmp(string, "0")
> 
> ?

let's use strncmp
Also, Options seems to allow for "false" and "False"

[Saam Barati]

Comment on attachment 320457 [details]
proposed patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=320457&action=review

>>> Source/JavaScriptCore/jsc.cpp:3846
>>> +    if (dumpOptionsOption && strcmp(dumpOptionsOption, "0")) {
>> 
>> OptionsOptionOptionsOptionsOptionOptions
>> 
>> Code seems to usually do:
>> !strcasecmp(string, "none") || !strcasecmp(string, "no") || !strcasecmp(string, "false") || !strcmp(string, "0")
>> 
>> ?
> 
> let's use strncmp
> Also, Options seems to allow for "false" and "False"

I think you should do this after Options::initialize so you can just write code like:
if (Options::dumpOptions()) ....

[Mark Lam]

Comment on attachment 320457 [details]
proposed patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=320457&action=review

>>>> Source/JavaScriptCore/jsc.cpp:3846
>>>> +    if (dumpOptionsOption && strcmp(dumpOptionsOption, "0")) {
>>> 
>>> OptionsOptionOptionsOptionsOptionOptions
>>> 
>>> Code seems to usually do:
>>> !strcasecmp(string, "none") || !strcasecmp(string, "no") || !strcasecmp(string, "false") || !strcmp(string, "0")
>>> 
>>> ?
>> 
>> let's use strncmp
>> Also, Options seems to allow for "false" and "False"
> 
> I think you should do this after Options::initialize so you can just write code like:
> if (Options::dumpOptions()) ....

I'll change to checking Options::dumpOptions() in CommandLine instead.  BTW, JSC_dumpOptions can only be set to an integer 0 to 3.  "false", "False", "none", etc are all invalid options.

[Mark Lam]

Created attachment 320469 [details]
proposed patch.

[Mark Lam]

Comment on attachment 320469 [details]
proposed patch.

Thanks for the review.

[WebKit Commit Bot]

Comment on attachment 320469 [details]
proposed patch.

Clearing flags on attachment: 320469

Committed r221878: <http://trac.webkit.org/changeset/221878>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/34693481>