552805 - Fix "overflow in implicit constant conversion" in TestEncoding.cpp

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Daniel Holbert [:dholbert]]

Attachment: fix

Filing this bug on fixing these spammy build warnings in TestEncoding.cpp:

> xpcom/tests/TestEncoding.cpp: In function ‘nsresult TestGoodSurrogatePair()’:
> xpcom/tests/TestEncoding.cpp:61: warning: overflow in implicit constant conversion
> xpcom/tests/TestEncoding.cpp:61: warning: overflow in implicit constant conversion
> xpcom/tests/TestEncoding.cpp:61: warning: overflow in implicit constant conversion
> xpcom/tests/TestEncoding.cpp:61: warning: overflow in implicit constant conversion
> xpcom/tests/TestEncoding.cpp: In function ‘nsresult TestBackwardsSurrogatePair()’:
> xpcom/tests/TestEncoding.cpp:105: warning: overflow in implicit constant conversion
> xpcom/tests/TestEncoding.cpp:105: warning: overflow in implicit constant conversion
> xpcom/tests/TestEncoding.cpp:105: warning: overflow in implicit constant conversion
> xpcom/tests/TestEncoding.cpp:105: warning: overflow in implicit constant conversion
> xpcom/tests/TestEncoding.cpp:105: warning: overflow in implicit constant conversion
> xpcom/tests/TestEncoding.cpp:105: warning: overflow in implicit constant conversion
> xpcom/tests/TestEncoding.cpp: In function ‘nsresult TestMalformedUTF16OrphanHighSurrogate()’:
> xpcom/tests/TestEncoding.cpp:148: warning: overflow in implicit constant conversion
> xpcom/tests/TestEncoding.cpp:148: warning: overflow in implicit constant conversion
> xpcom/tests/TestEncoding.cpp:148: warning: overflow in implicit constant conversion
> xpcom/tests/TestEncoding.cpp: In function ‘nsresult TestMalformedUTF16OrphanLowSurrogate()’:
> xpcom/tests/TestEncoding.cpp:191: warning: overflow in implicit constant conversion
> xpcom/tests/TestEncoding.cpp:191: warning: overflow in implicit constant conversion
> xpcom/tests/TestEncoding.cpp:191: warning: overflow in implicit constant conversion

This is because it does e.g.:
> 61   const char expected8[] = { 0xF0, 0x90, 0x8C, 0x82, 0x65, 0x78, 0x0 };

Values larger than 0x80 are out-of-bounds for a signed char, so it's warning us about the first four values in that array (and similar values in arrays later in the file).

We can fix this warning by...
 (a) casting each individual value to be unsigned, e.g. (unsigned char)0xF0
...or...
 (b) declaring the whole array as unsigned, "const unsigned char expected8[]"

I think (b) is simpler, though it requires a cast back to (const char*) later on, when we pass expected8 to a function that doesn't expect an unsigned char*.

I'm attaching a patch to do (b).  (Patch also fixes indentation to 2 spaces on the chunks I'm touching, to make them match the rest of the file.)

Requesting r=waldo, since he wrote the test.

Comment 1

[Jeff Walden [:Waldo]]

Comment on attachment 432931 [details] [diff] [review]
fix

Might be smaller to change the name and type of the array, then cast the array to a pointer to the not-unsigned type with the original name.  But this is fine as is.

Comment 2

[Daniel Holbert [:dholbert]]

(In reply to comment #1)
> Might be smaller to change the name and type of the array, then cast the array
> to a pointer to the not-unsigned type with the original name.

Yeah, I thought of that -- but I'm pretty sure it'd break all the sizeof(expected8) calls, if expected8 just became a pointer.  We'd have to change those to sizeof(actualUnderylingUnsignedArray), which would be a bit of a hack since the goal of this strategy would be to avoid directly accessing that that unsigned array.

> But this is fine
> as is.

Cool, thanks -- I'll just stick with the current patch then.

Comment 3

[Daniel Holbert [:dholbert]]

Pushed:
 http://hg.mozilla.org/mozilla-central/rev/64237e0343d3