Fix bad integer promotion in mysqlnd big5 charset

[adoy]

Fix bad integer promotion that causes big5 functions to always return the same result.

[cmb69]

Hmm, even if char is signed, isn't the cast to unsigned int well defined, and correct?

[Girgias]

We talked about it yesterday trying to understand why Sara's docker image to build releases doesn't compile for some people, and it seems the version of GCC which it currently uses (5.4.0) has a bug with the logical-op warning.

But I would also thing using zend_uchar as a cast is a bit strange here, a unsigned char cast makes more sense to me.

But I would also have expected that a char to unsigned int cast to be well defined

[adoy]

As @Girgias sayed I first started to look at this because Sara's docker reported me an error in this file on 8.1 and master (since the logical-op) warning was introduced.

workspace/php-src/ext/mysqlnd/mysqlnd_charset.c: In function 'check_mb_big5':
/workspace/php-src/ext/mysqlnd/mysqlnd_charset.c:191:54: error: logical 'and' of mutually exclusive tests is always false [-Werror=logical-op]
 #define valid_big5head(c) (0xA1 <= (unsigned int)(c) && (unsigned int)(c) <= 0xF9)
                                                      ^
/workspace/php-src/ext/mysqlnd/mysqlnd_charset.c:199:10: note: in expansion of macro 'valid_big5head'
  return (valid_big5head(*(start)) && (end - start) > 1 && valid_big5tail(*(start + 1)) ? 2 : 0);
          ^
/workspace/php-src/ext/mysqlnd/mysqlnd_charset.c:193:35: error: logical 'and' of mutually exclusive tests is always false [-Werror=logical-op]
        (0xA1 <= (unsigned int)(c) && (unsigned int)(c) <= 0xFE))
                                   ^
/workspace/php-src/ext/mysqlnd/mysqlnd_charset.c:199:59: note: in expansion of macro 'valid_big5tail'
  return (valid_big5head(*(start)) && (end - start) > 1 && valid_big5tail(*(start + 1)) ? 2 : 0);
                                                           ^
cc1: all warnings being treated as errors

It was strange because on my machine I didn't had the error. So I first looked at the gcc version. Mine is gcc (Debian 10.2.1-6) 10.2.1 20210110 and the one in the docker image is gcc (Ubuntu 5.4.0-6ubuntu1~16.04.12) 5.4.0 20160609.

To understand a bit more the problem I did some sample code to understand the problem and I came up with this :

int main() {
    char c = 0xA1;
    printf("%c %hhx\n", c, c);
    printf("char -> unsigned int %u\n", (unsigned int)(char) 0xA1);
    printf("char -> unsigned char -> int %u\n", (unsigned int)(unsigned char)(char) 0xA1);
    printf("unsigned int : %d\n", (unsigned int)(c) <= 0xF9);
    printf("unsigned char : %d\n", (unsigned char)(c) <= 0xF9);
}

output is the same on both versions :

� a1
char -> unsigned int 4294967201
char -> unsigned char -> unsigned int 161
unsigned int : 0
unsigned char : 1

So from my understanding when I convert an char to an unsigned it directly i got : a1 => ffffffa1 whereas if i use unsigned char a1 => a1 => a1.

[adoy]

@Girgias Juste to make sure

But I would also thing using zend_uchar as a cast is a bit strange here, a unsigned char cast makes more sense to me.

zend_uchar is unsigned char so did you mean it's strange to use zend_uchar or unsigned int ?

[cmb69]

So from my understanding when I convert an char to an unsigned it directly i got : a1 => ffffffa1 whereas if i use unsigned char a1 => a1 => a1.

Oh, right! I have no particular opinion regarding casting to unsigned char or zend_uchar().

[adoy]

@cmb69 So do you think the patch is valid ? Or am I still missing something ?

[Girgias]

@Girgias Juste to make sure

But I would also thing using zend_uchar as a cast is a bit strange here, a unsigned char cast makes more sense to me.

zend_uchar is unsigned char so did you mean it's strange to use zend_uchar or unsigned int ?

Erf I forgot zend_uchar is unsigned char...