From 2c62a90ddcc5d63c6bf0f080e5676bc4eed36655 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Fri, 6 Mar 2020 15:05:57 -0500 Subject: [PATCH] common: cast arguments to character classification functions to unsigned char We get failures of this type on the cygwin CI machine: 15:28:20 common.c: In function `bt_common_string_is_printable`: 15:28:20 common.c:786:16: error: array subscript has type `char` [-Werror=char-subscripts] 15:28:20 786 | if (!isprint(*ch) && *ch != '\n' && *ch != '\r' && 15:28:20 | ^~~ This error only pops up on some platforms that have isprint implemented using a lookup table. This table is indexed using `*ch`, which is a char. And because char is signed on some platforms, gcc warns that this is dangerous: we could access the array with a negative index, which would yield unexpected results. This is on purpose in newlib (the libc used by cygwin, apparently), see this comment: https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;a=blob;f=newlib/libc/include/ctype.h;h=a0009af17485acc3d70586a0051269a7a9c350d5;hb=HEAD#l78 The Linux man page for isprint also mentions it: The standards require that the argument c for these functions is either EOF or a value that is representable in the type unsigned char. If the argument c is of type char, it must be cast to unsigned char, as in the following example: char c; ... res = toupper((unsigned char) c); This is necessary because char may be the equivalent of signed char, in which case a byte where the top bit is set would be sign extended when converting to int, yielding a value that is outside the range of unsigned char. Add casts to unsigned char to fix the various instances of this error. Change-Id: Ice2305490997f595c6f5140a8be2abaa7fd1d8f6 Signed-off-by: Simon Marchi Reviewed-on: https://review.lttng.org/c/babeltrace/+/3194 Reviewed-by: Philippe Proulx CI-Build: Michael Jeanson Tested-by: jenkins (cherry picked from commit 994cd345db7a82957ce647c2dac28cae11382dcc) --- src/common/common.c | 2 +- src/plugins/ctf/common/metadata/lexer.l | 2 +- src/plugins/ctf/fs-sink/translate-trace-ir-to-ctf-ir.c | 4 ++-- src/plugins/text/dmesg/dmesg.c | 2 +- src/plugins/text/pretty/print.c | 2 +- tests/utils/tap/tap.c | 2 +- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/common/common.c b/src/common/common.c index 1a7d57cc..eed2451e 100644 --- a/src/common/common.c +++ b/src/common/common.c @@ -783,7 +783,7 @@ bool bt_common_string_is_printable(const char *input) BT_ASSERT_DBG(input); for (ch = input; *ch != '\0'; ch++) { - if (!isprint(*ch) && *ch != '\n' && *ch != '\r' && + if (!isprint((unsigned char) *ch) && *ch != '\n' && *ch != '\r' && *ch != '\t' && *ch != '\v') { printable = false; goto end; diff --git a/src/plugins/ctf/common/metadata/lexer.l b/src/plugins/ctf/common/metadata/lexer.l index 2dd45621..d0695c6b 100644 --- a/src/plugins/ctf/common/metadata/lexer.l +++ b/src/plugins/ctf/common/metadata/lexer.l @@ -134,5 +134,5 @@ _Imaginary setstring(yyextra, yylval, yytext); return CTF_IMAGINARY; {IDENTIFIER} BT_LOGT("Got identifier: id=\"%s\"", yytext); setstring(yyextra, yylval, yytext); if (is_type(yyextra, yytext)) return ID_TYPE; else return IDENTIFIER; [ \t\r\n] ; /* ignore */ -. _BT_LOGE_LINENO(yylineno, "Invalid character: char=\"%c\", val=0x%02x", isprint(yytext[0]) ? yytext[0] : '\0', yytext[0]); return CTF_ERROR; +. _BT_LOGE_LINENO(yylineno, "Invalid character: char=\"%c\", val=0x%02x", isprint((unsigned char) yytext[0]) ? yytext[0] : '\0', yytext[0]); return CTF_ERROR; %% diff --git a/src/plugins/ctf/fs-sink/translate-trace-ir-to-ctf-ir.c b/src/plugins/ctf/fs-sink/translate-trace-ir-to-ctf-ir.c index c808ef0a..e0a03b2e 100644 --- a/src/plugins/ctf/fs-sink/translate-trace-ir-to-ctf-ir.c +++ b/src/plugins/ctf/fs-sink/translate-trace-ir-to-ctf-ir.c @@ -154,14 +154,14 @@ bool ist_valid_identifier(const char *name) } /* Make sure the name starts with a letter or `_` */ - if (!isalpha(name[0]) && name[0] != '_') { + if (!isalpha((unsigned char) name[0]) && name[0] != '_') { ist_valid = false; goto end; } /* Make sure the name only contains letters, digits, and `_` */ for (at = name; *at != '\0'; at++) { - if (!isalnum(*at) && *at != '_') { + if (!isalnum((unsigned char) *at) && *at != '_') { ist_valid = false; goto end; } diff --git a/src/plugins/text/dmesg/dmesg.c b/src/plugins/text/dmesg/dmesg.c index 50ceb7e6..199dbf92 100644 --- a/src/plugins/text/dmesg/dmesg.c +++ b/src/plugins/text/dmesg/dmesg.c @@ -773,7 +773,7 @@ bt_message_iterator_class_next_method_status dmesg_msg_iter_next_one( /* Ignore empty lines, once trimmed */ for (ch = dmesg_msg_iter->linebuf; *ch != '\0'; ch++) { - if (!isspace(*ch)) { + if (!isspace((unsigned char) *ch)) { only_spaces = false; break; } diff --git a/src/plugins/text/pretty/print.c b/src/plugins/text/pretty/print.c index 92258030..f81b892b 100644 --- a/src/plugins/text/pretty/print.c +++ b/src/plugins/text/pretty/print.c @@ -639,7 +639,7 @@ void print_escape_string(struct pretty_component *pretty, const char *str) } /* Standard characters. */ - if (!iscntrl(str[i])) { + if (!iscntrl((unsigned char) str[i])) { bt_common_g_string_append_c(pretty->string, str[i]); continue; } diff --git a/tests/utils/tap/tap.c b/tests/utils/tap/tap.c index 7d5920ac..8e9f9a64 100644 --- a/tests/utils/tap/tap.c +++ b/tests/utils/tap/tap.c @@ -109,7 +109,7 @@ _gen_result(int ok, const char *func, const char *file, unsigned int line, if(local_test_name) { name_is_digits = 1; for(c = local_test_name; *c != '\0'; c++) { - if(!isdigit(*c) && !isspace(*c)) { + if(!isdigit((unsigned char) *c) && !isspace((unsigned char) *c)) { name_is_digits = 0; break; } -- 2.34.1