Am 13.05.2024 um 16:06 schrieb Taylor R Campbell: >> Module Name: src >> Committed By: rillig >> Date: Sun May 12 19:03:55 UTC 2024 >> >> Modified Files: >> src/usr.sbin/flashctl: flashctl.c >> >> Log Message: >> flashctl: fix lint's strict bool mode with Clang preprocessor >> >> Treating the return value from the <ctype.h> character classification >> functions as an 'int' is neither elegant nor idiomatic, but it works for >> now. >> >> - if (!isxdigit((unsigned char)str[2])) >> + if (isxdigit((unsigned char)str[2]) == 0) > > Why is this change necessary? Weren't you teaching lint to handle > this case without complaining?
Yes, I did. The thing is that I only ever tested lint's strict bool mode with the GCC preprocessor, and that preprocessor marks every token from a macro expansion as coming from a system header or coming from the user's code. Therefore, lint could be more lenient when checking the result of the isxdigit macro, as its main operator, the cast to '(int)', is from a system header, so it's OK that it has the "wrong" type. On the other hand, the strcmp function is not listed in "namespace.h" and does not have a macro definition, and although the function declaration comes from a system header, the GCC preprocessor does not mark the function call expression as coming from a system header. This is the difference by which lint treats 'isxdigit(ch)' as bool-like but 'strcmp(s1, s2)' as strictly 'int'. A few days or weeks ago, Christos started builds that have both MKLLVM=yes and MKLINT=yes, thus using the Clang preprocessor as lint's preprocessor. Apparently nobody else has done this in the last few years. Clang's preprocessor's output does not mark the tokens from macro expansions as coming from a system header or from the user's code. Due to this, lint can no longer be lenient for system header macro expansions as it doesn't get the system header information anymore. > We shouldn't change anything like > > if (!isxdigit(...)) > if (ferror(...)) > > to > > if (isxdigit(...) == 0) > if (ferror(...) != 0) > > The original is clearer and idiomatic code, even if it's a little > silly that the return value is declared as int and not bool > (presumably for historical reasons, if the interfaces were defined > before bool existed). I agree. For usr.bin/error, I tried a different variant, namely to only activate lint's strict bool mode when MKLLVM is undefined, thus when the GCC preprocessor is used. I guess activating the strict bool mode only in GCC mode is good enough to catch all type mismatches. The combination of MKLLVM=yes MKLINT=yes is also the cause why lint now allows do-while-0 even in strict bool mode, as the FD_ZERO macro uses that idiom and I didn't dare to change the macro to do-while-false, as that would either require <sys/stdbool.h>, or to deviate from the idiom by using 'do { ... } while (0 != 0)', as that would look suspicious. I will switch usr.sbin/flashctl to the only-in-GCC-mode variant and restore the previous idiomatic code. Roland