From 908071171b56a74b42fbb473ed648189f60388a0 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Tue, 25 Feb 2020 08:16:33 +0100 Subject: [PATCH] BUILD: general: always pass unsigned chars to is* functions The isalnum(), isalpha(), isdigit() etc functions from ctype.h are supposed to take an int in argument which must either reflect an unsigned char or EOF. In practice on some platforms they're implemented as macros referencing an array, and when passed a char, they either cause a warning "array subscript has type 'char'" when lucky, or cause random segfaults when unlucky. It's quite unconvenient by the way since none of them may return true for negative values. The recent introduction of cygwin to the list of regularly tested build platforms revealed a lot of breakage there due to the same issues again. So this patch addresses the problem all over the code at once. It adds unsigned char casts to every valid use case, and also drops the unneeded double cast to int that was sometimes added on top of it. It may be backported by dropping irrelevant changes if that helps better support uncommon platforms. It's unlikely to fix bugs on platforms which would already not emit any warning though. --- include/common/standard.h | 2 +- src/buffer.c | 2 +- src/cfgparse-listen.c | 10 +++++----- src/cfgparse.c | 8 ++++---- src/chunk.c | 4 ++-- src/fcgi-app.c | 2 +- src/fcgi.c | 2 +- src/flt_spoe.c | 22 +++++++++++----------- src/flt_trace.c | 2 +- src/haproxy.c | 2 +- src/hlua.c | 2 +- src/http_htx.c | 2 +- src/log.c | 2 +- src/sample.c | 4 ++-- src/server.c | 14 +++++++------- src/standard.c | 14 +++++++------- src/stats.c | 4 ++-- src/vars.c | 2 +- 18 files changed, 50 insertions(+), 50 deletions(-) diff --git a/include/common/standard.h b/include/common/standard.h index 6d2432f99..3d9083c1d 100644 --- a/include/common/standard.h +++ b/include/common/standard.h @@ -378,7 +378,7 @@ extern const char *invalid_prefix_char(const char *name); */ static inline int is_idchar(char c) { - return isalnum((int)(unsigned char)c) || + return isalnum((unsigned char)c) || c == '.' || c == '_' || c == '-' || c == '+' || c == ':'; } diff --git a/src/buffer.c b/src/buffer.c index c4d614051..6211d147c 100644 --- a/src/buffer.c +++ b/src/buffer.c @@ -85,7 +85,7 @@ void buffer_dump(FILE *o, struct buffer *b, int from, int to) } fprintf(o, " "); for (i = 0; (from + i < to) && (i < 16) ; i++) { - fprintf(o, "%c", isprint((int)b_orig(b)[from + i]) ? b_orig(b)[from + i] : '.') ; + fprintf(o, "%c", isprint((unsigned char)b_orig(b)[from + i]) ? b_orig(b)[from + i] : '.') ; if ((((from + i) & 15) == 15) && ((from + i) != to-1)) fprintf(o, "\n"); } diff --git a/src/cfgparse-listen.c b/src/cfgparse-listen.c index 3c2978c8a..bd2275238 100644 --- a/src/cfgparse-listen.c +++ b/src/cfgparse-listen.c @@ -1009,7 +1009,7 @@ int cfg_parse_listen(const char *file, int linenum, char **args, int kwm) } val = args[cur_arg + 1]; while (*val) { - if (iscntrl(*val) || *val == ';') { + if (iscntrl((unsigned char)*val) || *val == ';') { ha_alert("parsing [%s:%d]: character '%%x%02X' is not permitted in attribute value.\n", file, linenum, *val); err_code |= ERR_ALERT | ERR_FATAL; @@ -3649,11 +3649,11 @@ int cfg_parse_listen(const char *file, int linenum, char **args, int kwm) char *name, *end; name = args[cur_arg+1] + 7; - while (isspace(*name)) + while (isspace((unsigned char)*name)) name++; end = name; - while (*end && !isspace(*end) && *end != ',' && *end != ')') + while (*end && !isspace((unsigned char)*end) && *end != ',' && *end != ')') end++; curproxy->conn_src.opts &= ~CO_SRC_TPROXY_MASK; @@ -3665,14 +3665,14 @@ int cfg_parse_listen(const char *file, int linenum, char **args, int kwm) curproxy->conn_src.bind_hdr_occ = -1; /* now look for an occurrence number */ - while (isspace(*end)) + while (isspace((unsigned char)*end)) end++; if (*end == ',') { end++; name = end; if (*end == '-') end++; - while (isdigit((int)*end)) + while (isdigit((unsigned char)*end)) end++; curproxy->conn_src.bind_hdr_occ = strl2ic(name, end-name); } diff --git a/src/cfgparse.c b/src/cfgparse.c index 68cb0e980..cd3990494 100644 --- a/src/cfgparse.c +++ b/src/cfgparse.c @@ -374,7 +374,7 @@ int parse_process_number(const char *arg, unsigned long *proc, int max, int *aut for (p = arg; *p; p++) { if (*p == '-' && !dash) dash = p; - else if (!isdigit((int)*p)) { + else if (!isdigit((unsigned char)*p)) { memprintf(err, "'%s' is not a valid number/range.", arg); return -1; } @@ -420,7 +420,7 @@ unsigned long parse_cpu_set(const char **args, unsigned long *cpu_set, char **er char *dash; unsigned int low, high; - if (!isdigit((int)*args[cur_arg])) { + if (!isdigit((unsigned char)*args[cur_arg])) { memprintf(err, "'%s' is not a CPU range.\n", args[cur_arg]); return -1; } @@ -2035,13 +2035,13 @@ int readcfgfile(const char *file) braces = 1; } - if (!isalpha((int)(unsigned char)*var_beg) && *var_beg != '_') { + if (!isalpha((unsigned char)*var_beg) && *var_beg != '_') { ha_alert("parsing [%s:%d] : Variable expansion: Unrecognized character '%c' in variable name.\n", file, linenum, *var_beg); err_code |= ERR_ALERT | ERR_FATAL; goto next_line; /* skip current line */ } - while (isalnum((int)(unsigned char)*var_end) || *var_end == '_') + while (isalnum((unsigned char)*var_end) || *var_end == '_') var_end++; save_char = *var_end; diff --git a/src/chunk.c b/src/chunk.c index 8e77858c3..100783e23 100644 --- a/src/chunk.c +++ b/src/chunk.c @@ -193,7 +193,7 @@ int chunk_htmlencode(struct buffer *dst, struct buffer *src) c = src->area[i]; - if (!isascii(c) || !isprint((unsigned char)c) || c == '&' || c == '"' || c == '\'' || c == '<' || c == '>') { + if (!isascii((unsigned char)c) || !isprint((unsigned char)c) || c == '&' || c == '"' || c == '\'' || c == '<' || c == '>') { l = snprintf(dst->area + dst->data, free, "&#%u;", (unsigned char)c); @@ -235,7 +235,7 @@ int chunk_asciiencode(struct buffer *dst, struct buffer *src, char qc) c = src->area[i]; - if (!isascii(c) || !isprint((unsigned char)c) || c == '<' || c == '>' || c == qc) { + if (!isascii((unsigned char)c) || !isprint((unsigned char)c) || c == '<' || c == '>' || c == qc) { l = snprintf(dst->area + dst->data, free, "<%02X>", (unsigned char)c); diff --git a/src/fcgi-app.c b/src/fcgi-app.c index a9466a577..5d2658096 100644 --- a/src/fcgi-app.c +++ b/src/fcgi-app.c @@ -54,7 +54,7 @@ static struct ist fcgi_param_name(char *dst, const struct ist name) memcpy(dst, ":fcgi-", 6); ofs1 = 6; for (ofs2 = 0; ofs2 < name.len; ofs2++) { - if (isalnum((int)name.ptr[ofs2])) + if (isalnum((unsigned char)name.ptr[ofs2])) dst[ofs1++] = ist_lc[(unsigned char)name.ptr[ofs2]]; else dst[ofs1++] = '_'; diff --git a/src/fcgi.c b/src/fcgi.c index 33aa2a249..cb7b44e79 100644 --- a/src/fcgi.c +++ b/src/fcgi.c @@ -135,7 +135,7 @@ int fcgi_encode_param(struct buffer *out, const struct fcgi_param *p) } for (off = 0; off < p->n.len; off++) { - if (isalnum((int)p->n.ptr[off])) + if (isalnum((unsigned char)p->n.ptr[off])) out->area[len++] = ist_uc[(unsigned char)p->n.ptr[off]]; else out->area[len++] = '_'; diff --git a/src/flt_spoe.c b/src/flt_spoe.c index a78e6159e..ecad5c446 100644 --- a/src/flt_spoe.c +++ b/src/flt_spoe.c @@ -344,7 +344,7 @@ spoe_str_to_vsn(const char *str, size_t len) vsn = -1; /* skip leading spaces */ - while (p < end && isspace(*p)) + while (p < end && isspace((unsigned char)*p)) p++; /* parse Major number, until the '.' */ @@ -378,7 +378,7 @@ spoe_str_to_vsn(const char *str, size_t len) goto out; /* skip trailing spaces */ - while (p < end && isspace(*p)) + while (p < end && isspace((unsigned char)*p)) p++; if (p != end) goto out; @@ -784,21 +784,21 @@ spoe_handle_agenthello_frame(struct appctx *appctx, char *frame, size_t size) char *delim; /* Skip leading spaces */ - for (; isspace(*str) && sz; str++, sz--); + for (; isspace((unsigned char)*str) && sz; str++, sz--); if (sz >= 10 && !strncmp(str, "pipelining", 10)) { str += 10; sz -= 10; - if (!sz || isspace(*str) || *str == ',') + if (!sz || isspace((unsigned char)*str) || *str == ',') flags |= SPOE_APPCTX_FL_PIPELINING; } else if (sz >= 5 && !strncmp(str, "async", 5)) { str += 5; sz -= 5; - if (!sz || isspace(*str) || *str == ',') + if (!sz || isspace((unsigned char)*str) || *str == ',') flags |= SPOE_APPCTX_FL_ASYNC; } else if (sz >= 13 && !strncmp(str, "fragmentation", 13)) { str += 13; sz -= 13; - if (!sz || isspace(*str) || *str == ',') + if (!sz || isspace((unsigned char)*str) || *str == ',') flags |= SPOE_APPCTX_FL_FRAGMENTATION; } @@ -3587,7 +3587,7 @@ cfg_parse_spoe_agent(const char *file, int linenum, char **args, int kwm) goto out; tmp = args[2]; while (*tmp) { - if (!isalnum(*tmp) && *tmp != '_' && *tmp != '.') { + if (!isalnum((unsigned char)*tmp) && *tmp != '_' && *tmp != '.') { ha_alert("parsing [%s:%d]: '%s %s' only supports [a-zA-Z0-9_.] chars.\n", file, linenum, args[0], args[1]); err_code |= ERR_ALERT | ERR_FATAL; @@ -3621,7 +3621,7 @@ cfg_parse_spoe_agent(const char *file, int linenum, char **args, int kwm) goto out; tmp = args[2]; while (*tmp) { - if (!isalnum(*tmp) && *tmp != '_' && *tmp != '.') { + if (!isalnum((unsigned char)*tmp) && *tmp != '_' && *tmp != '.') { ha_alert("parsing [%s:%d]: '%s %s' only supports [a-zA-Z0-9_.] chars.\n", file, linenum, args[0], args[1]); err_code |= ERR_ALERT | ERR_FATAL; @@ -3645,7 +3645,7 @@ cfg_parse_spoe_agent(const char *file, int linenum, char **args, int kwm) goto out; tmp = args[2]; while (*tmp) { - if (!isalnum(*tmp) && *tmp != '_' && *tmp != '.') { + if (!isalnum((unsigned char)*tmp) && *tmp != '_' && *tmp != '.') { ha_alert("parsing [%s:%d]: '%s %s' only supports [a-zA-Z0-9_.] chars.\n", file, linenum, args[0], args[1]); err_code |= ERR_ALERT | ERR_FATAL; @@ -3669,7 +3669,7 @@ cfg_parse_spoe_agent(const char *file, int linenum, char **args, int kwm) goto out; tmp = args[2]; while (*tmp) { - if (!isalnum(*tmp) && *tmp != '_' && *tmp != '.') { + if (!isalnum((unsigned char)*tmp) && *tmp != '_' && *tmp != '.') { ha_alert("parsing [%s:%d]: '%s %s' only supports [a-zA-Z0-9_.] chars.\n", file, linenum, args[0], args[1]); err_code |= ERR_ALERT | ERR_FATAL; @@ -4185,7 +4185,7 @@ parse_spoe_flt(char **args, int *cur_arg, struct proxy *px, char *tmp = curagent->id; while (*tmp) { - if (!isalnum(*tmp) && *tmp != '_' && *tmp != '.') { + if (!isalnum((unsigned char)*tmp) && *tmp != '_' && *tmp != '.') { memprintf(err, "Invalid variable prefix '%s' for SPOE agent '%s' declared at %s:%d. " "Use 'option var-prefix' to set it. Only [a-zA-Z0-9_.] chars are supported.\n", curagent->id, curagent->id, curagent->conf.file, curagent->conf.line); diff --git a/src/flt_trace.c b/src/flt_trace.c index e349431a6..437f283a5 100644 --- a/src/flt_trace.c +++ b/src/flt_trace.c @@ -103,7 +103,7 @@ trace_hexdump(struct ist ist) if (i % 16 == 15) { fprintf(stderr, " |"); for(j = i - 15; j <= i && j < ist.len; j++) - fprintf(stderr, "%c", (isprint(*(ist.ptr+j)) ? *(ist.ptr+j) : '.')); + fprintf(stderr, "%c", (isprint((unsigned char)*(ist.ptr+j)) ? *(ist.ptr+j) : '.')); fprintf(stderr, "|\n"); } } diff --git a/src/haproxy.c b/src/haproxy.c index f04ccea6e..e23d28349 100644 --- a/src/haproxy.c +++ b/src/haproxy.c @@ -1680,7 +1680,7 @@ static void init(int argc, char **argv) *argv, strerror(errno)); exit(1); } else if (endptr && strlen(endptr)) { - while (isspace(*endptr)) endptr++; + while (isspace((unsigned char)*endptr)) endptr++; if (*endptr != 0) { ha_alert("-%2s option: some bytes unconsumed in PID list {%s}\n", flag, endptr); diff --git a/src/hlua.c b/src/hlua.c index 4c5598d8e..d8e538b63 100644 --- a/src/hlua.c +++ b/src/hlua.c @@ -838,7 +838,7 @@ static inline void hlua_sendlog(struct proxy *px, int level, const char *msg) *(p-1) = '.'; break; } - if (isprint(*msg)) + if (isprint((unsigned char)*msg)) *p = *msg; else *p = '.'; diff --git a/src/http_htx.c b/src/http_htx.c index f6e224305..d5239671b 100644 --- a/src/http_htx.c +++ b/src/http_htx.c @@ -1953,7 +1953,7 @@ int val_blk_arg(struct arg *arg, char **err_msg) int pos; for (pos = 0; pos < arg[0].data.str.data; pos++) { - if (!isdigit(arg[0].data.str.area[pos])) { + if (!isdigit((unsigned char)arg[0].data.str.area[pos])) { memprintf(err_msg, "invalid block position"); return 0; } diff --git a/src/log.c b/src/log.c index 40eac40a9..60b1a5a4d 100644 --- a/src/log.c +++ b/src/log.c @@ -666,7 +666,7 @@ int parse_logformat_string(const char *fmt, struct proxy *curproxy, struct list else { char c = *str; *str = 0; - if (isprint(c)) + if (isprint((unsigned char)c)) memprintf(err, "expected ']' after '%s', but found '%c'", var, c); else memprintf(err, "missing ']' after '%s'", var); diff --git a/src/sample.c b/src/sample.c index d3568708d..eb903ec6d 100644 --- a/src/sample.c +++ b/src/sample.c @@ -1466,7 +1466,7 @@ static int sample_conv_debug(const struct arg *arg_p, struct sample *smp, void * /* Display the displayable chars*. */ b_putchr(buf, '<'); for (i = 0; i < tmp.data.u.str.data; i++) { - if (isprint(tmp.data.u.str.area[i])) + if (isprint((unsigned char)tmp.data.u.str.area[i])) b_putchr(buf, tmp.data.u.str.area[i]); else b_putchr(buf, '.'); @@ -2046,7 +2046,7 @@ static int sample_conv_json(const struct arg *arg_p, struct sample *smp, void *p len = 2; str = "\\t"; } - else if (c > 0xff || !isprint(c)) { + else if (c > 0xff || !isprint((unsigned char)c)) { /* isprint generate a segfault if c is too big. The man says that * c must have the value of an unsigned char or EOF. */ diff --git a/src/server.c b/src/server.c index 959fb22e7..965570222 100644 --- a/src/server.c +++ b/src/server.c @@ -809,11 +809,11 @@ static int srv_parse_source(char **args, int *cur_arg, char *name, *end; name = args[*cur_arg + 1] + 7; - while (isspace(*name)) + while (isspace((unsigned char)*name)) name++; end = name; - while (*end && !isspace(*end) && *end != ',' && *end != ')') + while (*end && !isspace((unsigned char)*end) && *end != ',' && *end != ')') end++; newsrv->conn_src.opts &= ~CO_SRC_TPROXY_MASK; @@ -826,14 +826,14 @@ static int srv_parse_source(char **args, int *cur_arg, newsrv->conn_src.bind_hdr_occ = -1; /* now look for an occurrence number */ - while (isspace(*end)) + while (isspace((unsigned char)*end)) end++; if (*end == ',') { end++; name = end; if (*end == '-') end++; - while (isdigit((int)*end)) + while (isdigit((unsigned char)*end)) end++; newsrv->conn_src.bind_hdr_occ = strl2ic(name, end - name); } @@ -3434,7 +3434,7 @@ static void srv_state_parse_line(char *buf, const int version, char **params, ch } /* ignore blank characters at the beginning of the line */ - while (isspace(*cur)) + while (isspace((unsigned char)*cur)) ++cur; /* Ignore empty or commented lines */ @@ -3458,10 +3458,10 @@ static void srv_state_parse_line(char *buf, const int version, char **params, ch arg = 1; srv_arg = 0; while (*cur && arg < SRV_STATE_FILE_MAX_FIELDS) { - if (isspace(*cur)) { + if (isspace((unsigned char)*cur)) { *cur = '\0'; ++cur; - while (isspace(*cur)) + while (isspace((unsigned char)*cur)) ++cur; switch (version) { case 1: diff --git a/src/standard.c b/src/standard.c index 3c4081ebb..056710c67 100644 --- a/src/standard.c +++ b/src/standard.c @@ -586,7 +586,7 @@ const char *invalid_char(const char *name) return name; while (*name) { - if (!isalnum((int)(unsigned char)*name) && *name != '.' && *name != ':' && + if (!isalnum((unsigned char)*name) && *name != '.' && *name != ':' && *name != '_' && *name != '-') return name; name++; @@ -606,7 +606,7 @@ static inline const char *__invalid_char(const char *name, int (*f)(int)) { return name; while (*name) { - if (!f((int)(unsigned char)*name) && *name != '.' && + if (!f((unsigned char)*name) && *name != '.' && *name != '_' && *name != '-') return name; @@ -977,7 +977,7 @@ struct sockaddr_storage *str2sa_range(const char *str, int *port, int *low, int port1 = ""; } - if (isdigit((int)(unsigned char)*port1)) { /* single port or range */ + if (isdigit((unsigned char)*port1)) { /* single port or range */ port2 = strchr(port1, '-'); if (port2) *port2++ = '\0'; @@ -3820,7 +3820,7 @@ char *env_expand(char *in) var_beg++; var_end = var_beg; - while (isalnum((int)(unsigned char)*var_end) || *var_end == '_') { + while (isalnum((unsigned char)*var_end) || *var_end == '_') { var_end++; } @@ -4087,7 +4087,7 @@ int dump_text(struct buffer *out, const char *buf, int bsize) while (buf[ptr] && ptr < bsize) { c = buf[ptr]; - if (isprint(c) && isascii(c) && c != '\\' && c != ' ' && c != '=') { + if (isprint((unsigned char)c) && isascii((unsigned char)c) && c != '\\' && c != ' ' && c != '=') { if (out->data > out->size - 1) break; out->area[out->data++] = c; @@ -4183,7 +4183,7 @@ void dump_hex(struct buffer *out, const char *pfx, const void *buf, int len, int chunk_strcat(out, "'"); else if (unsafe > 1) chunk_strcat(out, "*"); - else if (isprint(d[i + j])) + else if (isprint((unsigned char)d[i + j])) chunk_appendf(out, "%c", d[i + j]); else chunk_strcat(out, "."); @@ -4214,7 +4214,7 @@ int dump_text_line(struct buffer *out, const char *buf, int bsize, int len, while (ptr < len && ptr < bsize) { c = buf[ptr]; - if (isprint(c) && isascii(c) && c != '\\') { + if (isprint((unsigned char)c) && isascii((unsigned char)c) && c != '\\') { if (out->data > end - 2) break; out->area[out->data++] = c; diff --git a/src/stats.c b/src/stats.c index f8a15bd54..28724fa2f 100644 --- a/src/stats.c +++ b/src/stats.c @@ -853,7 +853,7 @@ static int stats_dump_fields_html(struct buffer *out, if (flags & STAT_SHLGNDS) { chunk_appendf(out, "
"); - if (isdigit(*field_str(stats, ST_F_ADDR))) + if (isdigit((unsigned char)*field_str(stats, ST_F_ADDR))) chunk_appendf(out, "IPv4: %s, ", field_str(stats, ST_F_ADDR)); else if (*field_str(stats, ST_F_ADDR) == '[') chunk_appendf(out, "IPv6: %s, ", field_str(stats, ST_F_ADDR)); @@ -958,7 +958,7 @@ static int stats_dump_fields_html(struct buffer *out, if (flags & STAT_SHLGNDS) { chunk_appendf(out, "
"); - if (isdigit(*field_str(stats, ST_F_ADDR))) + if (isdigit((unsigned char)*field_str(stats, ST_F_ADDR))) chunk_appendf(out, "IPv4: %s, ", field_str(stats, ST_F_ADDR)); else if (*field_str(stats, ST_F_ADDR) == '[') chunk_appendf(out, "IPv6: %s, ", field_str(stats, ST_F_ADDR)); diff --git a/src/vars.c b/src/vars.c index 63e210331..403cb69f1 100644 --- a/src/vars.c +++ b/src/vars.c @@ -271,7 +271,7 @@ static char *register_name(const char *name, int len, enum vars_scope *scope, /* Check variable name syntax. */ tmp = var_names[var_names_nb - 1]; while (*tmp) { - if (!isalnum((int)(unsigned char)*tmp) && *tmp != '_' && *tmp != '.') { + if (!isalnum((unsigned char)*tmp) && *tmp != '_' && *tmp != '.') { memprintf(err, "invalid syntax at char '%s'", tmp); res = NULL; goto end;