Skip to content

Commit

Permalink
common: cast arguments to character classification functions to unsig…
Browse files Browse the repository at this point in the history
…ned 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 <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/3194
Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
CI-Build: Michael Jeanson <mjeanson@efficios.com>
Tested-by: jenkins <jenkins@lttng.org>
(cherry picked from commit 994cd34)
  • Loading branch information
simark authored and jgalar committed Mar 10, 2020
1 parent 6559432 commit 2c62a90
Show file tree
Hide file tree
Showing 6 changed files with 7 additions and 7 deletions.
2 changes: 1 addition & 1 deletion src/common/common.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/ctf/common/metadata/lexer.l
Original file line number Diff line number Diff line change
Expand Up @@ -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;
%%
4 changes: 2 additions & 2 deletions src/plugins/ctf/fs-sink/translate-trace-ir-to-ctf-ir.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/text/dmesg/dmesg.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/text/pretty/print.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion tests/utils/tap/tap.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down

0 comments on commit 2c62a90

Please sign in to comment.