On 17.05.2017 23:24, Andrew Morton wrote: > On Wed, 17 May 2017 15:33:53 +0200 Kamil Rytarowski <n54@xxxxxxx> wrote: > >> The current code produces set of warnings on NetBSD-7.99.25 (GCC 4.8.5): >> >> In file included from scripts/kconfig/zconf.tab.c:2576:0: >> scripts/kconfig/confdata.c: In function 'conf_expand_value': >> scripts/kconfig/confdata.c:97:3: >> warning: array subscript has type 'char' [-Wchar-subscripts] >> while (isalnum(*src) || *src == '_') >> ^ >> scripts/kconfig/confdata.c: In function 'conf_set_sym_val': >> scripts/kconfig/confdata.c:155:4: >> warning: array subscript has type 'char' [-Wchar-subscripts] >> for (p2 = p; *p2 && !isspace(*p2); p2++) >> ^ >> scripts/kconfig/confdata.c: In function 'tristate_print_symbol': >> scripts/kconfig/confdata.c:617:3: >> warning: array subscript has type 'char' [-Wchar-subscripts] >> fprintf(fp, "%s%s=%c\n", CONFIG_, sym->name, (char)toupper(*value)); >> ^ >> >> Fix this portability issue by explicit casting to unsigned char. >> >> ... >> >> scripts/basic/fixdep.c | 2 +- >> scripts/kconfig/conf.c | 6 +++--- >> scripts/kconfig/confdata.c | 9 +++++---- >> scripts/kconfig/expr.c | 3 ++- >> scripts/kconfig/lxdialog/checklist.c | 3 ++- >> scripts/kconfig/lxdialog/inputbox.c | 3 ++- >> scripts/kconfig/lxdialog/menubox.c | 11 +++++++---- >> scripts/kconfig/lxdialog/util.c | 5 +++-- >> scripts/kconfig/menu.c | 4 ++-- >> scripts/kconfig/nconf.c | 3 ++- >> scripts/kconfig/nconf.gui.c | 3 ++- >> scripts/kconfig/symbol.c | 8 ++++---- >> 12 files changed, 35 insertions(+), 25 deletions(-) > > This patch is irritating :( The Linux manpage says > > "These functions check whether c, which must have the value of an > unsigned char or EOF..." > > so it's legit enough. > > Apart from being ugly, it isn't very maintainable: people on Linux > systems will add new instances while not including the typecasts. > I proposed this casting as it's a common idiom in portable software using ctype(3) in C. Examples in GNU software (I used "grep -r 'unsigned char'" and randomly picked some sequential lines): gpl2/gmake/dist/glob/fnmatch.c: if ((STREQ (str, "alnum") && ISALNUM ((unsigned char) *n)) gpl2/gmake/dist/glob/fnmatch.c: || (STREQ (str, "alpha") && ISALPHA ((unsigned char) *n)) gpl2/gmake/dist/glob/fnmatch.c: || (STREQ (str, "blank") && ISBLANK ((unsigned char) *n)) gpl2/gmake/dist/glob/fnmatch.c: || (STREQ (str, "cntrl") && ISCNTRL ((unsigned char) *n)) gpl2/gmake/dist/glob/fnmatch.c: || (STREQ (str, "digit") && ISDIGIT ((unsigned char) *n)) gpl2/gmake/dist/glob/fnmatch.c: || (STREQ (str, "graph") && ISGRAPH ((unsigned char) *n)) gpl2/gmake/dist/glob/fnmatch.c: || (STREQ (str, "lower") && ISLOWER ((unsigned char) *n)) gpl2/gmake/dist/glob/fnmatch.c: || (STREQ (str, "print") && ISPRINT ((unsigned char) *n)) gpl2/gmake/dist/glob/fnmatch.c: || (STREQ (str, "punct") && ISPUNCT ((unsigned char) *n)) gpl2/gmake/dist/glob/fnmatch.c: || (STREQ (str, "space") && ISSPACE ((unsigned char) *n)) gpl2/gmake/dist/glob/fnmatch.c: || (STREQ (str, "upper") && ISUPPER ((unsigned char) *n)) gpl2/gmake/dist/glob/fnmatch.c: || (STREQ (str, "xdigit") && ISXDIGIT ((unsigned char) *n))) gpl2/groff/dist/src/libs/libgroff/strtol.c: while (ISASCII((unsigned char)*str) && isspace((unsigned char)*str)) gpl2/groff/dist/src/libs/libgroff/strtol.c: p = strchr(digits, (ISASCII((unsigned char)*str) gpl2/groff/dist/src/libs/libgroff/strtol.c: && isupper((unsigned char)*str) gpl2/groff/dist/src/libs/libgroff/strtol.c: ? tolower((unsigned char)*str) gpl2/gettext/dist/gettext-runtime/intl/l10nflist.c: if (isalpha ((unsigned char) codeset[cnt])) gpl2/gettext/dist/gettext-runtime/intl/l10nflist.c: *wp++ = tolower ((unsigned char) codeset[cnt]); gpl2/gettext/dist/gettext-runtime/intl/l10nflist.c: else if (isdigit ((unsigned char) codeset[cnt])) gpl2/gettext/dist/gettext-runtime/intl/localealias.c: while (isspace ((unsigned char) cp[0])) gpl2/gettext/dist/gettext-runtime/intl/localealias.c: while (cp[0] != '\0' && !isspace ((unsigned char) cp[0])) gpl2/gettext/dist/gettext-runtime/intl/localealias.c: while (isspace ((unsigned char) cp[0])) I also noted it already in the Linux kernel related software (the one I got under hand to grep): gpl2/dtc/dist/dtc-lexer.l: while (!isdigit((unsigned char)*line)) gpl2/dtc/dist/dtc-lexer.l: while (!isspace((unsigned char)*tmp)) I put aside discussion about prettiness of C. > I wonder if there's anothing we can do in the Makefiles to suppress > that warning in scripts/ for netbsd systems? ie, disable > -Wchar-subscripts in that case? > This is another option, to hide warnings. As of now "menuconfig" does not crash, however I haven't tested it on signed vs unsigned char type platforms. LIBC implementations on Linux are generally more tolerant to passing values out of valid range and hide bugs, that sometimes crash with our native LIBC... and from time to time some people try to "fix" our compliant implementation: http://gnats.netbsd.org/34632 "isalpha() and possibly other ctype functions segfault"
Attachment:
signature.asc
Description: OpenPGP digital signature
