From: Willy Tarreau Date: Tue, 25 Feb 2020 07:16:33 +0000 (+0100) Subject: BUILD: general: always pass unsigned chars to is* functions X-Git-Tag: v2.2-dev3~21 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=908071171b56a74b42fbb473ed648189f60388a0;p=thirdparty%2Fhaproxy.git 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. --- diff --git a/include/common/standard.h b/include/common/standard.h index 6d2432f991..3d9083c1dc 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 c4d6140513..6211d147c5 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 3c2978c8ae..bd22752387 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 @@ stats_error_parsing: 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 @@ stats_error_parsing: 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 68cb0e9803..cd39904947 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 @@ next_line: 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 8e77858c3a..100783e238 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 a9466a5770..5d26580965 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 33aa2a2491..cb7b44e792 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 a78e6159e7..ecad5c4461 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 e349431a69..437f283a50 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 f04ccea6ef..e23d283499 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 4c5598d8e2..d8e538b637 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 f6e224305f..d5239671b0 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 40eac40a99..60b1a5a4d7 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 d3568708df..eb903ec6d3 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 959fb22e74..9655702227 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 3c4081ebb6..056710c672 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 f8a15bd547..28724fa2f0 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 63e2103311..403cb69f1d 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;