Re: [RFC PATCH 6/6] utf8.c: avoid char overflow

Junio C Hamano [off-list ref] writes:

Beat Bolli [off-list ref] writes:

quoted

quoted

quoted

-static const char utf16_be_bom[] = {0xFE, 0xFF};
-static const char utf16_le_bom[] = {0xFF, 0xFE};
-static const char utf32_be_bom[] = {0x00, 0x00, 0xFE, 0xFF};
-static const char utf32_le_bom[] = {0xFF, 0xFE, 0x00, 0x00};
+static const unsigned char utf16_be_bom[] = {0xFE, 0xFF};
+static const unsigned char utf16_le_bom[] = {0xFF, 0xFE};
+static const unsigned char utf32_be_bom[] = {0x00, 0x00, 0xFE, 0xFF};
+static const unsigned char utf32_le_bom[] = {0xFF, 0xFE, 0x00, 0x00};

An alternative approach that might be easier to read (and avoids the
confusion arising from our use of (signed) chars for strings pretty
much
everywhere):

#define FE ((char)0xfe)
#define FF ((char)0xff)

...

I have tried this first (without the macros, though), and thought
it looked really ugly. That's why I chose this solution. The usage
is pretty local and close to function has_bom_prefix().

I found that what you posted was already OK, as has_bom_prefix()
appears only locally in this file and that is the only thing that
cares about these foo_bom[] constants.  Casting the elements in
these arrays to (char) type is also fine and not all that ugly,
I think, and between the two (but without the macro) I have no
strong preference.  I wonder if writing them as '\376' and '\377'
as old timers would helps the compiler, though.

Heh, I should have read the remainder of my mailbox to learn that
you came to the same conclusion in response to Dscho's comment
before saying the above.  Hex is OK as all our compilers would be
capable of gloking '\xff' (there is a rather unusual '\x20' already
in builtin/clone.c).

Thanks.