Fix likely misuse of ctype.h API #4397

	The standard C <ctype.h> API takes type int as argument, but when
that argument value is neither (a) a value representable by type unsigned
char nor (b) the value of the macro EOF, the behaviour is undefined. See
C11, Sec. 7.4 Character handling <ctype.h> p.200, clause 1.

	The <ctype.h> functions are designed to take as arguments the
values returned by getc or fgetc, no for processing elements of an
arbitrary string stored in a char array. Safely processing arbitrary
strings requires explicit cast to unsigned char to keep the argument
values within the domain {EOF, 0, 1 ..., 255}.

	OTH, passing negative values {-128, -127, ..., -1} may trigger
undefined behaviour, and also the non-EOF 0xff can be conflated with -1,
which, although not forbidden, may give unintended results.

	This commit introduces an explicit cast to unsigned char when
passing argument to the standard <ctype.h>, except when that argument
was properly taken from the return of getc, fgetc, fgets and so on.

Reported-by: riastradh
Signed-off-by: Thales Antunes de Oliveira Barretto <[email protected]>

api/src/glfs.c

@@ -850,7 +850,7 @@ pub_glfs_new(const char *volname)
     }
 
     for (i = 0; i < strlen(volname); i++) {
-        if (!isalnum(volname[i]) && (volname[i] != '_') &&
+        if (!isalnum((unsigned char)volname[i]) && (volname[i] != '_') &&
             (volname[i] != '-')) {
             errno = EINVAL;
             return NULL;

cli/src/cli-cmd-parser.c

@@ -562,7 +562,7 @@ cli_validate_volname(const char *volname)
     }
 
     for (i = 0; i < volname_len; i++) {
-        if (!isalnum(volname[i]) && (volname[i] != '_') &&
+        if (!isalnum((unsigned char)volname[i]) && (volname[i] != '_') &&
             (volname[i] != '-')) {
             cli_err(
                 "Volume name should not contain \"%c\""
@@ -1702,7 +1702,7 @@ gf_strip_whitespace(char *str, int len)
         return -1;
 
     for (i = 0; i < len; i++) {
-        if (!isspace(str[i]))
+        if (!isspace((unsigned char)str[i]))
             new_str[new_len++] = str[i];
     }
     new_str[new_len] = '\0';
@@ -3087,7 +3087,7 @@ gf_is_str_int(const char *value)
     fptr = str;
 
     while (*str) {
-        if (!isdigit(*str)) {
+        if (!isdigit((unsigned char)*str)) {
             flag = 1;
             goto out;
         }
@@ -4173,7 +4173,7 @@ cli_snap_clone_parse(dict_t *dict, const char **words, int wordcount)
     clonename = (char *)words[cmdi];
     for (i = 0; i < strlen(clonename); i++) {
         /* Following volume name convention */
-        if (!isalnum(clonename[i]) &&
+        if (!isalnum((unsigned char)clonename[i]) &&
             (clonename[i] != '_' && (clonename[i] != '-'))) {
             /* TODO : Is this message enough?? */
             cli_err(
@@ -4255,7 +4255,7 @@ cli_snap_create_parse(dict_t *dict, const char **words, int wordcount)
     snapname = (char *)words[cmdi];
     for (i = 0; i < strlen(snapname); i++) {
         /* Following volume name convention */
-        if (!isalnum(snapname[i]) &&
+        if (!isalnum((unsigned char)snapname[i]) &&
             (snapname[i] != '_' && (snapname[i] != '-'))) {
             /* TODO : Is this message enough?? */
             cli_err(
@@ -5468,7 +5468,7 @@ cli_cmd_validate_volume(char *volname)
     }
 
     for (i = 0; i < volname_len; i++)
-        if (!isalnum(volname[i]) && (volname[i] != '_') &&
+        if (!isalnum((unsigned char)volname[i]) && (volname[i] != '_') &&
             (volname[i] != '-')) {
             cli_err(
                 "Volume name should not contain \"%c\""

cli/src/cli-xml-output.c

@@ -3412,7 +3412,7 @@ _output_gsync_config(FILE *fp, xmlTextWriterPtr writer, char *op_name)
             break;
 
         v = resbuf + strlen(resbuf) - 1;
-        while (isspace(*v)) {
+        while (isspace((unsigned char)*v)) {
             /* strip trailing space */
             *v-- = '\0';
         }
@@ -3434,7 +3434,7 @@ _output_gsync_config(FILE *fp, xmlTextWriterPtr writer, char *op_name)
             goto out;
         }
         *v++ = '\0';
-        while (isspace(*v))
+        while (isspace((unsigned char)*v))
             v++;
         v = gf_strdup(v);
         if (!v) {

extras/benchmarking/rdd.c

@@ -209,7 +209,7 @@ string2bytesize(const char *str, unsigned long long *n)
     }
 
     for (s = str; *s != '\0'; s++) {
-        if (isspace(*s)) {
+        if (isspace((unsigned char)*s)) {
             continue;
         }
         if (*s == '-') {

extras/geo-rep/gsync-sync-gfid.c

@@ -62,7 +62,7 @@ main(int argc, char *argv[])
 
         path += UUID_CANONICAL_FORM_LEN + 1;
 
-        while (isspace(*path))
+        while (isspace((unsigned char)*path))
             path++;
 
         len = strlen(line);

geo-replication/src/procdiggy.c

@@ -55,7 +55,7 @@ pidinfo(pid_t pid, char **name)
         if (name && !*name) {
             p = strtail(buf, "Name:");
             if (p) {
-                while (isspace(*++p))
+                while (isspace((unsigned char)*++p))
                     ;
                 *name = gf_strdup(p);
                 if (!*name) {
@@ -71,7 +71,7 @@ pidinfo(pid_t pid, char **name)
             break;
     }
 
-    while (isspace(*++p))
+    while (isspace((unsigned char)*++p))
         ;
     ret = gf_string2int(p, &lpid);
     if (ret == -1)

libglusterfs/src/common-utils.c

@@ -713,14 +713,14 @@ gf_trim(char *string)
         return NULL;
     }
 
-    for (s = string; isspace(*s); s++)
+    for (s = string; isspace((unsigned char)*s); s++)
         ;
 
     if (*s == 0)
         return s;
 
     t = s + strlen(s) - 1;
-    while (t > s && isspace(*t))
+    while (t > s && isspace((unsigned char)*t))
         t--;
     *++t = '\0';
 
@@ -778,7 +778,7 @@ gf_string2time(const char *str, time_t *n)
     }
 
     for (s = str; *s != '\0'; s++) {
-        if (isspace(*s))
+        if (isspace((unsigned char)*s))
             continue;
         if (*s == '-')
             return -1;
@@ -852,7 +852,7 @@ gf_string2percent(const char *str, double *n)
     }
 
     for (s = str; *s != '\0'; s++) {
-        if (isspace(*s))
+        if (isspace((unsigned char)*s))
             continue;
         if (*s == '-')
             return -1;
@@ -928,7 +928,7 @@ _gf_string2ulong(const char *str, unsigned long *n, int base)
     }
 
     for (s = str; *s != '\0'; s++) {
-        if (isspace(*s))
+        if (isspace((unsigned char)*s))
             continue;
         if (*s == '-')
             return -1;
@@ -971,7 +971,7 @@ _gf_string2uint(const char *str, unsigned int *n, int base)
     }
 
     for (s = str; *s != '\0'; s++) {
-        if (isspace(*s))
+        if (isspace((unsigned char)*s))
             continue;
         if (*s == '-')
             return -1;
@@ -1082,7 +1082,7 @@ _gf_string2ulonglong(const char *str, unsigned long long *n, int base)
     }
 
     for (s = str; *s != '\0'; s++) {
-        if (isspace(*s))
+        if (isspace((unsigned char)*s))
             continue;
         if (*s == '-')
             return -1;
@@ -1450,7 +1450,7 @@ gf_string2bytesize_range(const char *str, uint64_t *n, uint64_t umax)
     max = umax & 0x7fffffffffffffffLL;
 
     for (s = str; *s != '\0'; s++) {
-        if (isspace(*s))
+        if (isspace((unsigned char)*s))
             continue;
         if (*s == '-')
             return -1;
@@ -1549,7 +1549,7 @@ gf_string2percent_or_bytesize(const char *str, double *n,
     }
 
     for (s = str; *s != '\0'; s++) {
-        if (isspace(*s))
+        if (isspace((unsigned char)*s))
             continue;
         if (*s == '-')
             return -1;
@@ -1806,7 +1806,7 @@ strtail(char *str, const char *pattern)
 void
 skipwhite(char **s)
 {
-    while (isspace(**s))
+    while (isspace((unsigned char)**s))
         (*s)++;
 }
 
@@ -1995,7 +1995,8 @@ valid_host_name(char *address, int length)
         goto out;
     }
 
-    if (!isalnum(dup_addr[length - 1]) && (dup_addr[length - 1] != '*')) {
+    if (!isalnum((unsigned char)dup_addr[length - 1]) &&
+        (dup_addr[length - 1] != '*')) {
         ret = 0;
         goto out;
     }
@@ -2013,12 +2014,13 @@ valid_host_name(char *address, int length)
     do {
         str_len = strlen(temp_str);
 
-        if (!isalnum(temp_str[0]) || !isalnum(temp_str[str_len - 1])) {
+        if (!isalnum((unsigned char)temp_str[0]) ||
+            !isalnum((unsigned char)temp_str[str_len - 1])) {
             ret = 0;
             goto out;
         }
         for (i = 1; i < str_len; i++) {
-            if (!isalnum(temp_str[i]) && (temp_str[i] != '-')) {
+            if (!isalnum((unsigned char)temp_str[i]) && (temp_str[i] != '-')) {
                 ret = 0;
                 goto out;
             }
@@ -2049,7 +2051,8 @@ valid_ipv4_address(char *address, int length, gf_boolean_t wildcard_acc)
      * delimiters.
      */
     if (length <= 0 || (strstr(address, "..")) ||
-        (!isdigit(tmp[length - 1]) && (tmp[length - 1] != '*'))) {
+        (!isdigit((unsigned char)tmp[length - 1]) &&
+         (tmp[length - 1] != '*'))) {
         ret = 0;
         goto out;
     }

tests/basic/open-behind/tester.c

@@ -82,7 +82,8 @@ buffer_get(context_t *ctx)
 static int32_t
 str_skip_spaces(context_t *ctx, int32_t current)
 {
-    while ((current > 0) && (current != '\n') && isspace(current)) {
+    while ((current > 0) && (current != '\n') &&
+           isspace((unsigned char)current)) {
         current = buffer_get(ctx);
     }
 
@@ -98,7 +99,7 @@ str_token(context_t *ctx, char *buffer, uint32_t size, int32_t current)
 
     len = 0;
     while ((size > 0) && (current > 0) && (current != '\n') &&
-           !isspace(current)) {
+           !isspace((unsigned char)current)) {
         len++;
         *buffer++ = current;
         size--;

xlators/cluster/ec/src/ec-code.c

@@ -838,7 +838,7 @@ ec_code_proc_trim_left(char *text, ssize_t *length)
 {
     ssize_t len;
 
-    for (len = *length; (len > 0) && isspace(*text); len--) {
+    for (len = *length; (len > 0) && isspace((unsigned char)*text); len--) {
         text++;
     }
     *length = len;
@@ -856,7 +856,7 @@ ec_code_proc_trim_right(char *text, ssize_t *length, char sep)
 
     last = text;
     for (len = *length; (len > 0) && (*text != sep); len--) {
-        if (!isspace(*text)) {
+        if (!isspace((unsigned char)*text)) {
             last = text + 1;
         }
         text++;

xlators/debug/io-stats/src/io-stats.c

@@ -822,7 +822,7 @@ io_stats_dump_global_to_json_logfp(xlator_t *this,
     for (i = 0; i < GF_FOP_MAXVALUE; i++) {
         lc_fop_name = strdupa(gf_fop_list[i]);
         for (j = 0; lc_fop_name[j]; j++) {
-            lc_fop_name[j] = tolower(lc_fop_name[j]);
+            lc_fop_name[j] = tolower((unsigned char)lc_fop_name[j]);
         }
 
         fop_hits = GF_ATOMIC_GET(stats->fop_hits[i]);
@@ -879,7 +879,7 @@ io_stats_dump_global_to_json_logfp(xlator_t *this,
     for (i = 0; i < GF_UPCALL_FLAGS_MAXVALUE; i++) {
         lc_fop_name = strdupa(gf_upcall_list[i]);
         for (j = 0; lc_fop_name[j]; j++) {
-            lc_fop_name[j] = tolower(lc_fop_name[j]);
+            lc_fop_name[j] = tolower((unsigned char)lc_fop_name[j]);
         }
         fop_hits = GF_ATOMIC_GET(stats->upcall_hits[i]);
         if (interval == -1) {

xlators/mgmt/glusterd/src/glusterd-ganesha.c

@@ -83,7 +83,8 @@ parsing_ganesha_ha_conf(const char *key)
         do {
             end_pointer++;
         } while (!(*end_pointer == '\'' || *end_pointer == '"' ||
-                   isspace(*end_pointer) || *end_pointer == '\0'));
+                   isspace((unsigned char)*end_pointer) ||
+                   *end_pointer == '\0'));
         *end_pointer = '\0';
 
         /* got it. copy it and return */

xlators/mgmt/glusterd/src/glusterd-geo-rep.c

@@ -760,7 +760,7 @@ _fcbk_conftodict(char *resbuf, size_t blen, FILE *fp, void *data)
         if (!ptr)
             break;
         v = resbuf + strlen(resbuf) - 1;
-        while (isspace(*v))
+        while (isspace((unsigned char)*v))
             /* strip trailing space */
             *v-- = '\0';
         if (v == resbuf)
@@ -770,7 +770,7 @@ _fcbk_conftodict(char *resbuf, size_t blen, FILE *fp, void *data)
         if (!v)
             return -1;
         *v++ = '\0';
-        while (isspace(*v))
+        while (isspace((unsigned char)*v))
             v++;
         v = gf_strdup(v);
         if (!v)
@@ -826,7 +826,7 @@ _fcbk_statustostruct(char *resbuf, size_t blen, FILE *fp, void *data)
             break;
 
         v = resbuf + strlen(resbuf) - 1;
-        while (isspace(*v))
+        while (isspace((unsigned char)*v))
             /* strip trailing space */
             *v-- = '\0';
         if (v == resbuf)
@@ -836,7 +836,7 @@ _fcbk_statustostruct(char *resbuf, size_t blen, FILE *fp, void *data)
         if (!v)
             return -1;
         *v++ = '\0';
-        while (isspace(*v))
+        while (isspace((unsigned char)*v))
             v++;
         v = gf_strdup(v);
         if (!v)
@@ -4427,7 +4427,7 @@ glusterd_gsync_read_frm_status(char *path, char *buf, size_t blen)
             ret = -1;
         } else {
             char *p = buf + len - 1;
-            while (isspace(*p))
+            while (isspace((unsigned char)*p))
                 *p-- = '\0';
         }
     } else if (ret == 0)

xlators/mgmt/glusterd/src/glusterd-mountbroker.c

@@ -140,7 +140,7 @@ parse_mount_pattern_desc(gf_mount_spec_t *mspec, char *pdesc)
                 ret = SYNTAX_ERR;
                 goto out;
             }
-            while (!strchr("|&)", *c2) && !isspace(*c2))
+            while (!strchr("|&)", *c2) && !isspace((unsigned char)*c2))
                 c2++;
             skipwhite(&c2);
             switch (*c2) {
@@ -182,7 +182,7 @@ parse_mount_pattern_desc(gf_mount_spec_t *mspec, char *pdesc)
         c2 = ""; /* reset c2 */
         while (*c2 != ')') {
             c2 = curs;
-            while (!isspace(*c2) && *c2 != ')')
+            while (!isspace((unsigned char)*c2) && *c2 != ')')
                 c2++;
             sc = *c2;
             *c2 = '\0';

xlators/mgmt/glusterd/src/glusterd-pmap.c

@@ -150,7 +150,7 @@ pmap_registry_search(xlator_t *this, char *brickname, gf_boolean_t destroy)
     {
         brck = tmp_port->brickname;
         for (;;) {
-            for (i = 0; brck[i] && !isspace(brck[i]); ++i)
+            for (i = 0; brck[i] && !isspace((unsigned char)brck[i]); ++i)
                 ;
             if (i == 0 && brck[i] == '\0')
                 break;
@@ -175,7 +175,7 @@ pmap_registry_search(xlator_t *this, char *brickname, gf_boolean_t destroy)
              * Skip over *any* amount of whitespace, including
              * none (if we're already at the end of the string).
              */
-            while (isspace(*brck))
+            while (isspace((unsigned char)*brck))
                 ++brck;
             /*
              * We're either at the end of the string (which will be

xlators/mgmt/glusterd/src/glusterd-utils.c

@@ -6927,7 +6927,7 @@ glusterd_parse_inode_size(char *stream, char *pattern)
     needle = nwstrtail(needle, pattern);
 
     trail = needle;
-    while (trail && isdigit(*trail))
+    while (trail && isdigit((unsigned char)*trail))
         trail++;
     if (trail)
         *trail = '\0';