Fix multicharacter constant issues

[franksinankaya]

@am11 @jkotas @janvorli

[filipnavara]

What is the rationale? It is valid C99 (section 6.4.4.4) and I was convinced it's also valid C++ (https://en.cppreference.com/w/cpp/language/character_literal) although I have not checked the specification.

[franksinankaya]

There is some more information here:
https://stackoverflow.com/questions/7755202/multi-character-constant-warnings

"The value of an integer character constant containing more than one character (e.g., 'ab'), [...] is implementation-defined."

[franksinankaya]

Warning is seen with gcc.

[filipnavara]

It is implementation-defined but still very much valid syntax. I have yet to see an implementation that doesn't translate it as 4-byte integer where each byte represents one character. At least in C99 mode gcc didn't warn me about it unless I explicitly turned on the warnings or used -pedantic.

[franksinankaya]

It is probably 4 bytes in general but there is an issue with endianness.

From Wikipedia:

Multi-character constants (e.g. 'xy') are valid, although rarely useful — they let one store several characters in an integer (e.g. 4 ASCII characters can fit in a 32-bit integer, 8 in a 64-bit one). Since the order in which the characters are packed into one int is not specified, portable use of multi-character constants is difficult.

[filipnavara]

...but there is an issue with endianness.

Generally this is valid concern. Then again, CoreCLR works only on little-endian machines and both GCC and Clang interpret it the same way.

[franksinankaya]

I got your point. We are seeing a compilation error with GCC at this moment.

We have already fixed similar warnings in the past.
The workaround to not produce a warning is much more uglier than this change itself and unmanageable.

This change itself is also removing a compatibility issue. It is a win-win in the end.

Not taking it right now means we need to create a tech-debt for someone to cleanup in the future.

[franksinankaya]

Also, I need to emphasize that this is the only place in entire code base where this warning is seen.

[mikedn]

This should probably just use memcmp or something like that, the current approach is likely unnecessary "optimization".

E.g.

if (memcmp(buffer + 4, "GenuineIntel", 12) == 0) {
...

All used C++ compilers transform this memcmp into a couple of integer compares.

[jkotas]

LGTM modulo two nits.