Skip to content

Commit ec86019

Browse files
ThalesBarrettoThalesBarretto
committed
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. Casting to unsigned char avoids sign extension when implicitly converting to int. This commit introduces an explicit cast to unsigned char when passing argument to the standard <ctype.h> function family. Reported-by: riastradh Signed-off-by: Thales Antunes de Oliveira Barretto <[email protected]> # api/src/glfs.c | 2 +- # cli/src/cli-cmd-parser.c | 12 ++++----- # cli/src/cli-xml-output.c | 4 +-- # extras/benchmarking/rdd.c | 2 +- # extras/geo-rep/gsync-sync-gfid.c | 2 +- # geo-replication/src/procdiggy.c | 4 +-- # libglusterfs/src/common-utils.c | 31 +++++++++++++----------- # tests/basic/open-behind/tester.c | 5 ++-- # xlators/cluster/ec/src/ec-code.c | 4 +-- # xlators/debug/io-stats/src/io-stats.c | 4 +-- # xlators/mgmt/glusterd/src/glusterd-ganesha.c | 5 ++-- # xlators/mgmt/glusterd/src/glusterd-geo-rep.c | 10 ++++---- # xlators/mgmt/glusterd/src/glusterd-mountbroker.c | 4 +-- # xlators/mgmt/glusterd/src/glusterd-pmap.c | 4 +-- # xlators/mgmt/glusterd/src/glusterd-utils.c | 2 +- # xlators/mgmt/glusterd/src/glusterd-volume-set.c | 3 ++- # xlators/performance/md-cache/src/md-cache.c | 2 +- # 17 files changed, 53 insertions(+), 47 deletions(-) # api/src/glfs.c | 2 +- # cli/src/cli-cmd-parser.c | 12 ++++----- # cli/src/cli-xml-output.c | 4 +-- # extras/benchmarking/rdd.c | 2 +- # extras/geo-rep/gsync-sync-gfid.c | 2 +- # geo-replication/src/procdiggy.c | 4 +-- # libglusterfs/src/common-utils.c | 31 +++++++++++++----------- # tests/basic/open-behind/tester.c | 5 ++-- # xlators/cluster/ec/src/ec-code.c | 4 +-- # xlators/debug/io-stats/src/io-stats.c | 4 +-- # xlators/mgmt/glusterd/src/glusterd-ganesha.c | 5 ++-- # xlators/mgmt/glusterd/src/glusterd-geo-rep.c | 10 ++++---- # xlators/mgmt/glusterd/src/glusterd-mountbroker.c | 4 +-- # xlators/mgmt/glusterd/src/glusterd-pmap.c | 4 +-- # xlators/mgmt/glusterd/src/glusterd-utils.c | 2 +- # xlators/mgmt/glusterd/src/glusterd-volume-set.c | 3 ++- # xlators/performance/md-cache/src/md-cache.c | 2 +- # 17 files changed, 53 insertions(+), 47 deletions(-) # api/src/glfs.c | 2 +- # cli/src/cli-cmd-parser.c | 12 ++++----- # cli/src/cli-xml-output.c | 4 +-- # extras/benchmarking/rdd.c | 2 +- # extras/geo-rep/gsync-sync-gfid.c | 2 +- # geo-replication/src/procdiggy.c | 4 +-- # libglusterfs/src/common-utils.c | 31 +++++++++++++----------- # tests/basic/open-behind/tester.c | 5 ++-- # xlators/cluster/ec/src/ec-code.c | 4 +-- # xlators/debug/io-stats/src/io-stats.c | 4 +-- # xlators/mgmt/glusterd/src/glusterd-ganesha.c | 5 ++-- # xlators/mgmt/glusterd/src/glusterd-geo-rep.c | 10 ++++---- # xlators/mgmt/glusterd/src/glusterd-mountbroker.c | 4 +-- # xlators/mgmt/glusterd/src/glusterd-pmap.c | 4 +-- # xlators/mgmt/glusterd/src/glusterd-utils.c | 2 +- # xlators/mgmt/glusterd/src/glusterd-volume-set.c | 3 ++- # xlators/performance/md-cache/src/md-cache.c | 2 +- # 17 files changed, 53 insertions(+), 47 deletions(-)
1 parent c440132 commit ec86019

File tree

17 files changed

+53
-47
lines changed

17 files changed

+53
-47
lines changed

‎api/src/glfs.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -850,7 +850,7 @@ pub_glfs_new(const char *volname)
850850
}
851851

852852
for (i = 0; i < strlen(volname); i++) {
853-
if (!isalnum(volname[i]) && (volname[i] != '_') &&
853+
if (!isalnum((unsigned char)volname[i]) && (volname[i] != '_') &&
854854
(volname[i] != '-')) {
855855
errno = EINVAL;
856856
return NULL;

‎cli/src/cli-cmd-parser.c‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -562,7 +562,7 @@ cli_validate_volname(const char *volname)
562562
}
563563

564564
for (i = 0; i < volname_len; i++) {
565-
if (!isalnum(volname[i]) && (volname[i] != '_') &&
565+
if (!isalnum((unsigned char)volname[i]) && (volname[i] != '_') &&
566566
(volname[i] != '-')) {
567567
cli_err(
568568
"Volume name should not contain \"%c\""
@@ -1702,7 +1702,7 @@ gf_strip_whitespace(char *str, int len)
17021702
return -1;
17031703

17041704
for (i = 0; i < len; i++) {
1705-
if (!isspace(str[i]))
1705+
if (!isspace((unsigned char)str[i]))
17061706
new_str[new_len++] = str[i];
17071707
}
17081708
new_str[new_len] = '\0';
@@ -3087,7 +3087,7 @@ gf_is_str_int(const char *value)
30873087
fptr = str;
30883088

30893089
while (*str) {
3090-
if (!isdigit(*str)) {
3090+
if (!isdigit((unsigned char)*str)) {
30913091
flag = 1;
30923092
goto out;
30933093
}
@@ -4173,7 +4173,7 @@ cli_snap_clone_parse(dict_t *dict, const char **words, int wordcount)
41734173
clonename = (char *)words[cmdi];
41744174
for (i = 0; i < strlen(clonename); i++) {
41754175
/* Following volume name convention */
4176-
if (!isalnum(clonename[i]) &&
4176+
if (!isalnum((unsigned char)clonename[i]) &&
41774177
(clonename[i] != '_' && (clonename[i] != '-'))) {
41784178
/* TODO : Is this message enough?? */
41794179
cli_err(
@@ -4255,7 +4255,7 @@ cli_snap_create_parse(dict_t *dict, const char **words, int wordcount)
42554255
snapname = (char *)words[cmdi];
42564256
for (i = 0; i < strlen(snapname); i++) {
42574257
/* Following volume name convention */
4258-
if (!isalnum(snapname[i]) &&
4258+
if (!isalnum((unsigned char)snapname[i]) &&
42594259
(snapname[i] != '_' && (snapname[i] != '-'))) {
42604260
/* TODO : Is this message enough?? */
42614261
cli_err(
@@ -5468,7 +5468,7 @@ cli_cmd_validate_volume(char *volname)
54685468
}
54695469

54705470
for (i = 0; i < volname_len; i++)
5471-
if (!isalnum(volname[i]) && (volname[i] != '_') &&
5471+
if (!isalnum((unsigned char)volname[i]) && (volname[i] != '_') &&
54725472
(volname[i] != '-')) {
54735473
cli_err(
54745474
"Volume name should not contain \"%c\""

‎cli/src/cli-xml-output.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3412,7 +3412,7 @@ _output_gsync_config(FILE *fp, xmlTextWriterPtr writer, char *op_name)
34123412
break;
34133413

34143414
v = resbuf + strlen(resbuf) - 1;
3415-
while (isspace(*v)) {
3415+
while (isspace((unsigned char)*v)) {
34163416
/* strip trailing space */
34173417
*v-- = '\0';
34183418
}
@@ -3434,7 +3434,7 @@ _output_gsync_config(FILE *fp, xmlTextWriterPtr writer, char *op_name)
34343434
goto out;
34353435
}
34363436
*v++ = '\0';
3437-
while (isspace(*v))
3437+
while (isspace((unsigned char)*v))
34383438
v++;
34393439
v = gf_strdup(v);
34403440
if (!v) {

‎extras/benchmarking/rdd.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ string2bytesize(const char *str, unsigned long long *n)
209209
}
210210

211211
for (s = str; *s != '\0'; s++) {
212-
if (isspace(*s)) {
212+
if (isspace((unsigned char)*s)) {
213213
continue;
214214
}
215215
if (*s == '-') {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ main(int argc, char *argv[])
6262

6363
path += UUID_CANONICAL_FORM_LEN + 1;
6464

65-
while (isspace(*path))
65+
while (isspace((unsigned char)*path))
6666
path++;
6767

6868
len = strlen(line);

‎geo-replication/src/procdiggy.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ pidinfo(pid_t pid, char **name)
5555
if (name && !*name) {
5656
p = strtail(buf, "Name:");
5757
if (p) {
58-
while (isspace(*++p))
58+
while (isspace((unsigned char)*++p))
5959
;
6060
*name = gf_strdup(p);
6161
if (!*name) {
@@ -71,7 +71,7 @@ pidinfo(pid_t pid, char **name)
7171
break;
7272
}
7373

74-
while (isspace(*++p))
74+
while (isspace((unsigned char)*++p))
7575
;
7676
ret = gf_string2int(p, &lpid);
7777
if (ret == -1)

‎libglusterfs/src/common-utils.c‎

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -713,14 +713,14 @@ gf_trim(char *string)
713713
return NULL;
714714
}
715715

716-
for (s = string; isspace(*s); s++)
716+
for (s = string; isspace((unsigned char)*s); s++)
717717
;
718718

719719
if (*s == 0)
720720
return s;
721721

722722
t = s + strlen(s) - 1;
723-
while (t > s && isspace(*t))
723+
while (t > s && isspace((unsigned char)*t))
724724
t--;
725725
*++t = '\0';
726726

@@ -778,7 +778,7 @@ gf_string2time(const char *str, time_t *n)
778778
}
779779

780780
for (s = str; *s != '\0'; s++) {
781-
if (isspace(*s))
781+
if (isspace((unsigned char)*s))
782782
continue;
783783
if (*s == '-')
784784
return -1;
@@ -852,7 +852,7 @@ gf_string2percent(const char *str, double *n)
852852
}
853853

854854
for (s = str; *s != '\0'; s++) {
855-
if (isspace(*s))
855+
if (isspace((unsigned char)*s))
856856
continue;
857857
if (*s == '-')
858858
return -1;
@@ -928,7 +928,7 @@ _gf_string2ulong(const char *str, unsigned long *n, int base)
928928
}
929929

930930
for (s = str; *s != '\0'; s++) {
931-
if (isspace(*s))
931+
if (isspace((unsigned char)*s))
932932
continue;
933933
if (*s == '-')
934934
return -1;
@@ -971,7 +971,7 @@ _gf_string2uint(const char *str, unsigned int *n, int base)
971971
}
972972

973973
for (s = str; *s != '\0'; s++) {
974-
if (isspace(*s))
974+
if (isspace((unsigned char)*s))
975975
continue;
976976
if (*s == '-')
977977
return -1;
@@ -1082,7 +1082,7 @@ _gf_string2ulonglong(const char *str, unsigned long long *n, int base)
10821082
}
10831083

10841084
for (s = str; *s != '\0'; s++) {
1085-
if (isspace(*s))
1085+
if (isspace((unsigned char)*s))
10861086
continue;
10871087
if (*s == '-')
10881088
return -1;
@@ -1450,7 +1450,7 @@ gf_string2bytesize_range(const char *str, uint64_t *n, uint64_t umax)
14501450
max = umax & 0x7fffffffffffffffLL;
14511451

14521452
for (s = str; *s != '\0'; s++) {
1453-
if (isspace(*s))
1453+
if (isspace((unsigned char)*s))
14541454
continue;
14551455
if (*s == '-')
14561456
return -1;
@@ -1549,7 +1549,7 @@ gf_string2percent_or_bytesize(const char *str, double *n,
15491549
}
15501550

15511551
for (s = str; *s != '\0'; s++) {
1552-
if (isspace(*s))
1552+
if (isspace((unsigned char)*s))
15531553
continue;
15541554
if (*s == '-')
15551555
return -1;
@@ -1806,7 +1806,7 @@ strtail(char *str, const char *pattern)
18061806
void
18071807
skipwhite(char **s)
18081808
{
1809-
while (isspace(**s))
1809+
while (isspace((unsigned char)**s))
18101810
(*s)++;
18111811
}
18121812

@@ -1995,7 +1995,8 @@ valid_host_name(char *address, int length)
19951995
goto out;
19961996
}
19971997

1998-
if (!isalnum(dup_addr[length - 1]) && (dup_addr[length - 1] != '*')) {
1998+
if (!isalnum((unsigned char)dup_addr[length - 1]) &&
1999+
(dup_addr[length - 1] != '*')) {
19992000
ret = 0;
20002001
goto out;
20012002
}
@@ -2013,12 +2014,13 @@ valid_host_name(char *address, int length)
20132014
do {
20142015
str_len = strlen(temp_str);
20152016

2016-
if (!isalnum(temp_str[0]) || !isalnum(temp_str[str_len - 1])) {
2017+
if (!isalnum((unsigned char)temp_str[0]) ||
2018+
!isalnum((unsigned char)temp_str[str_len - 1])) {
20172019
ret = 0;
20182020
goto out;
20192021
}
20202022
for (i = 1; i < str_len; i++) {
2021-
if (!isalnum(temp_str[i]) && (temp_str[i] != '-')) {
2023+
if (!isalnum((unsigned char)temp_str[i]) && (temp_str[i] != '-')) {
20222024
ret = 0;
20232025
goto out;
20242026
}
@@ -2049,7 +2051,8 @@ valid_ipv4_address(char *address, int length, gf_boolean_t wildcard_acc)
20492051
* delimiters.
20502052
*/
20512053
if (length <= 0 || (strstr(address, "..")) ||
2052-
(!isdigit(tmp[length - 1]) && (tmp[length - 1] != '*'))) {
2054+
(!isdigit((unsigned char)tmp[length - 1]) &&
2055+
(tmp[length - 1] != '*'))) {
20532056
ret = 0;
20542057
goto out;
20552058
}

‎tests/basic/open-behind/tester.c‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,8 @@ buffer_get(context_t *ctx)
8282
static int32_t
8383
str_skip_spaces(context_t *ctx, int32_t current)
8484
{
85-
while ((current > 0) && (current != '\n') && isspace(current)) {
85+
while ((current > 0) && (current != '\n') &&
86+
isspace((unsigned char)current)) {
8687
current = buffer_get(ctx);
8788
}
8889

@@ -98,7 +99,7 @@ str_token(context_t *ctx, char *buffer, uint32_t size, int32_t current)
9899

99100
len = 0;
100101
while ((size > 0) && (current > 0) && (current != '\n') &&
101-
!isspace(current)) {
102+
!isspace((unsigned char)current)) {
102103
len++;
103104
*buffer++ = current;
104105
size--;

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -838,7 +838,7 @@ ec_code_proc_trim_left(char *text, ssize_t *length)
838838
{
839839
ssize_t len;
840840

841-
for (len = *length; (len > 0) && isspace(*text); len--) {
841+
for (len = *length; (len > 0) && isspace((unsigned char)*text); len--) {
842842
text++;
843843
}
844844
*length = len;
@@ -856,7 +856,7 @@ ec_code_proc_trim_right(char *text, ssize_t *length, char sep)
856856

857857
last = text;
858858
for (len = *length; (len > 0) && (*text != sep); len--) {
859-
if (!isspace(*text)) {
859+
if (!isspace((unsigned char)*text)) {
860860
last = text + 1;
861861
}
862862
text++;

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -822,7 +822,7 @@ io_stats_dump_global_to_json_logfp(xlator_t *this,
822822
for (i = 0; i < GF_FOP_MAXVALUE; i++) {
823823
lc_fop_name = strdupa(gf_fop_list[i]);
824824
for (j = 0; lc_fop_name[j]; j++) {
825-
lc_fop_name[j] = tolower(lc_fop_name[j]);
825+
lc_fop_name[j] = tolower((unsigned char)lc_fop_name[j]);
826826
}
827827

828828
fop_hits = GF_ATOMIC_GET(stats->fop_hits[i]);
@@ -879,7 +879,7 @@ io_stats_dump_global_to_json_logfp(xlator_t *this,
879879
for (i = 0; i < GF_UPCALL_FLAGS_MAXVALUE; i++) {
880880
lc_fop_name = strdupa(gf_upcall_list[i]);
881881
for (j = 0; lc_fop_name[j]; j++) {
882-
lc_fop_name[j] = tolower(lc_fop_name[j]);
882+
lc_fop_name[j] = tolower((unsigned char)lc_fop_name[j]);
883883
}
884884
fop_hits = GF_ATOMIC_GET(stats->upcall_hits[i]);
885885
if (interval == -1) {

0 commit comments

Comments
 (0)