298286 - Clean up js_ReportCompileErrorNumber to deal with jschar *

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Blake Kaplan (:mrbkap) (inactive)]

Currently, js_ReportCompileErrorNumber() unconditionally calls
js_ExpandErrorArguments with its charArgs parameter set to PR_TRUE, which causes
problems for jsregexp.c which consistently calls it with jschar* arguments. This
breaks because interpreting the jschar* as a char* results in one-character-only
strings.

I'm volunteering to fix this in a sane way.

Comment 1

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: patch v1

This adds an argument to js_ReportCompileErrorNumber, which bloats the patch
somewhat. The alternative patch (which would probably be a bit cleaner) would
be to add a function js_ReportCompileErrorNumberUC [akin to
JS_ReportErrorNumber(UC)] and split the common code into another function. I
think I actually like the alternate better. Brendan, what do you think?

Comment 2

[Brendan Eich [:brendan]]

Comment on attachment 187682 [details] [diff] [review]
patch v1

Yeah, let's do the UC thing (per office discussion).  Law of least effort has
not been repealed. ;-)

/be

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: patch v2

Using report as a pointer inflated this patch some, but I think this is as
close as I can get to combining the two functions.

Comment 4

[Brendan Eich [:brendan]]

Comment on attachment 187847 [details] [diff] [review]
patch v2

Looks good to me -- if it all tests well, go for 1.8b4 approval -- I'll help
approve.  Thanks,

/be

Comment 5

[Brendan Eich [:brendan]]

Comment on attachment 187847 [details] [diff] [review]
patch v2

This is safe for 1.8b4.

/be

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

Checked in.

Comment 7

[Bob Clary [:bc] (inactive)]

testcase-, suggestions welcome.