search

dgibson / dtc

Device Tree Compiler
235 stars 134 forks source link

Char subscript error in Cygwin #71

Open longr96 opened 2 years ago

longr96 commented 2 years ago

I'm getting the following error when building in Cygwin

checks.c:1183:52: error: array subscript has type 'char' [-Werror=char-subscripts] 1183 | if (unitname[0] == '0' && isxdigit(unitname[1]))

I guess all that needs to be done is to do a type cast on the '1'?

dgibson commented 2 years ago

Hmm... if an integer literal can't be used as an array subscript that really seems like a bug with the compiler rather than the code in question.

joelsherrill commented 1 year ago

I think you are misreading the warning. isxdigit() takes an integer argument and unitname[0] is a char. This is very deliberately a warning by GCC and llvm. This is from the GCC manual:

-Wchar-subscripts Warn if an array subscript has type char. This is a common cause of error, as programmers often forget that this type is signed on some machines. This warning is enabled by -Wall.

The warning should be fixed.

dgibson commented 1 year ago

I think you are misreading the warning. isxdigit() takes an integer argument and unitname[0] is a char. This is very deliberately a warning by GCC and llvm. This is from the GCC manual:

So, yes, I misread the error as about being the apparent subscripts here rather than the isxdigit() call, but the description of the error still doesn't make the problem apparent.

-Wchar-subscripts Warn if an array subscript has type char. This is a common cause of error, as programmers often forget that this type is signed on some machines. This warning is enabled by -Wall.

Yes, we're expecting unitname[1] to be promoted to int as a parameter to the function isxdigit(), but that doesn't really have anything to do with array subscripts. My best guess here is that the Cygwin isxdigit() is implemented as a macro. If that macro is then indexing a lookup table using the parameter, that would explain this error. If that's the case, I really think that's a bug in the isxdigit() implementation, rather than the code in dtc - although it takes an int for (borderline archaic) technical reasons, isxdigit() is explicitly about testing characters, it seems pretty bogus for it not to be ok to pass a char value to it.

Fwiw, I always compile with -Wall on Linux and I've never seen this error, nor do I see it if I explicitly add -Wchar-subscripts.

The warning should be fixed.

I'm guessing you mean by making it isxdigit((int)unitname[1])?

I'm not thrilled at the prospect of obfuscating the code with an additional cast to work around what's arguably a bug in the library implementation.

sebhub commented 1 year ago

The basic problem is that it is implementation-defined if char is signed or unsigned. See also https://stackoverflow.com/questions/17975913/does-ctype-h-still-require-unsigned-char.

I think all the is(x) calls from ctype.h should be changed to is((unsigned char)x).