Closed Bug 644753 Opened 9 years ago Closed 6 years ago

Turn jschar ILLEGAL_RANGE NS_ASSERTIONS to warnings

Categories

(Core :: XPConnect, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla5

People

(Reporter: cdleary, Assigned: cdleary)

References

(Blocks 1 open bug)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 file)

Prevents some valid activities (i.e. writing binary data read as x-user-defined charset to file through an OutputStream) from triggering a just-in-time debugging breakpoint on Windows.
Attachment #521622 - Flags: review?(mrbkap)
Comment on attachment 521622 [details] [diff] [review]
Tame conversion assertion.

>diff --git a/js/src/xpconnect/src/xpcconvert.cpp b/js/src/xpconnect/src/xpcconvert.cpp
>+#ifdef DEBUG
>+static bool
>+CheckJSCharInCharRange(jschar c)
>+{
>+    if (ILLEGAL_RANGE(c)) {

Nit: style in this file is:

if(ILLEGAL_RANGE(c))
{
...
}

>+        /* U+0080/U+0100 - U+FFFF data lost. */
>+        static const size_t MSG_BUF_SIZE = 64;
>+        char msg[MSG_BUF_SIZE];
>+        JS_snprintf(msg, MSG_BUF_SIZE, "jschar out of char range; high bits of data lost: 0x%x", c);
>+        NS_WARNING(msg);

Is it worth doing NS_WARNING if the high bits are ff and NS_ERROR otherwise? That way, the sign extension case gets warnings and everybody else gets errors. If you don't think it's worth it, it isn't a big deal.

>-                  if(ILLEGAL_RANGE(*t))
>-                      legalRange = PR_FALSE;
>+                    if (!CheckJSCharInCharRange(*t))

No space after the |if| here, either.

r=me with that addressed.
Attachment #521622 - Flags: review?(mrbkap) → review+
Mass-reassigning cdleary's bugs to default. He won't work on any of them, anymore. I guess, at least.

@cdleary: shout if you take issue with this.
Assignee: cdleary → nobody
Status: ASSIGNED → NEW
https://hg.mozilla.org/mozilla-central/rev/85158bd8698d
Assignee: nobody → cdleary
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla5
You need to log in before you can comment on or make changes to this bug.