Skip to content

Commit f8c28fc

Browse files
bwbarrettbwbarrett
committed
class/cstring: Fix handling of corner cases
Fix the bool interpretation, which had a bug that made it impossible to interpret the strings "false" or "no" properly. Fix issues uncovered with handling overflow and underflow of the int type when writing the test. Add a test for the opal_cstring interface. Signed-off-by: Brian Barrett <[email protected]> (cherry picked from commit 4f42204)
1 parent 1fbe5c1 commit f8c28fc

File tree

5 files changed

+285
-19
lines changed

5 files changed

+285
-19
lines changed

‎.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -633,6 +633,7 @@ test/class/ompi_rb_tree
633633
test/class/ompi_bitmap
634634
test/class/opal_bitmap
635635
test/class/opal_fifo
636+
test/class/opal_cstring
636637
test/class/opal_hash_table
637638
test/class/opal_lifo
638639
test/class/opal_list

‎opal/class/opal_cstring.c

Lines changed: 29 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ static void opal_cstring_ctor(opal_cstring_t *obj)
3333
{
3434
*(size_t *) &(obj->length) = 0;
3535
/* make sure the string is null-terminated */
36-
((char *) obj->string)[0] = '\n';
36+
((char *) obj->string)[0] = '\0';
3737
}
3838

3939
static inline size_t opal_cstring_alloc_size(size_t len)
@@ -90,10 +90,18 @@ int opal_cstring_to_int(opal_cstring_t *string, int *interp)
9090
if (*endp != '\0') {
9191
return OPAL_ERR_BAD_PARAM;
9292
}
93-
/* underflow */
93+
/* base errors */
9494
if (tmp == 0 && errno == EINVAL) {
9595
return OPAL_ERR_BAD_PARAM;
9696
}
97+
/* overflow/underflow of long int (return of strtol) */
98+
if (errno == ERANGE && (tmp == LONG_MIN || tmp == LONG_MAX)) {
99+
return OPAL_ERR_BAD_PARAM;
100+
}
101+
/* overflow/underflow of int (for cases where long int is larger) */
102+
if (tmp < INT_MIN || tmp > INT_MAX) {
103+
return OPAL_ERR_BAD_PARAM;
104+
}
97105

98106
*interp = (int) tmp;
99107

@@ -104,24 +112,28 @@ static int opal_str_to_bool_impl(const char *string, bool *interp)
104112
{
105113
const char *ptr = string;
106114

107-
/* Trim leading whitespace */
108-
while (isspace(*ptr)) {
109-
++ptr;
110-
}
115+
if (NULL != string) {
116+
/* Trim leading whitespace */
117+
while (isspace(*ptr)) {
118+
++ptr;
119+
}
111120

112-
if ('\0' != *ptr) {
113-
if (isdigit(*ptr)) {
114-
*interp = (bool) atoi(ptr);
115-
} else if (0 == strncasecmp(ptr, "yes", 3) || 0 == strncasecmp(ptr, "true", 4)) {
116-
*interp = true;
117-
} else if (0 == strncasecmp(ptr, "no", 2) && 0 == strncasecmp(ptr, "false", 5)) {
118-
*interp = false;
119-
} else {
120-
*interp = false;
121-
return OPAL_ERR_BAD_PARAM;
121+
if ('\0' != *ptr) {
122+
if (isdigit(*ptr)) {
123+
*interp = (bool) atoi(ptr);
124+
return OPAL_SUCCESS;
125+
} else if (0 == strncasecmp(ptr, "yes", 3) || 0 == strncasecmp(ptr, "true", 4)) {
126+
*interp = true;
127+
return OPAL_SUCCESS;
128+
} else if (0 == strncasecmp(ptr, "no", 2) || 0 == strncasecmp(ptr, "false", 5)) {
129+
*interp = false;
130+
return OPAL_SUCCESS;
131+
}
122132
}
123133
}
124-
return OPAL_SUCCESS;
134+
135+
*interp = false;
136+
return OPAL_ERR_BAD_PARAM;
125137
}
126138

127139
int opal_cstring_to_bool(opal_cstring_t *string, bool *interp)

‎opal/class/opal_cstring.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@
4343
#ifndef OPAL_STRING_H
4444
#define OPAL_STRING_H
4545

46-
#include "opal_config.h"
4746
#include "opal/class/opal_object.h"
4847
#include "opal/mca/base/mca_base_var_enum.h"
4948

‎test/class/Makefile.am

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@ check_PROGRAMS = \
3535
opal_value_array \
3636
opal_pointer_array \
3737
opal_lifo \
38-
opal_fifo
38+
opal_fifo \
39+
opal_cstring
3940

4041
TESTS = $(check_PROGRAMS)
4142

@@ -94,6 +95,12 @@ opal_fifo_LDADD = \
9495
$(top_builddir)/test/support/libsupport.a
9596
opal_fifo_DEPENDENCIES = $(opal_fifo_LDADD)
9697

98+
opal_cstring_SOURCES = opal_cstring.c
99+
opal_cstring_LDADD = \
100+
$(top_builddir)/opal/lib@[email protected] \
101+
$(top_builddir)/test/support/libsupport.a
102+
opal_cstring_DEPENDENCIES = $(opal_cstring_LDADD)
103+
97104
clean-local:
98105
rm -f opal_bitmap_test_out.txt opal_hash_table_test_out.txt opal_proc_table_test_out.txt
99106

‎test/class/opal_cstring.c

Lines changed: 247 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,247 @@
1+
/*
2+
* Copyright (c) 2022 Amazon.com, Inc. or its affiliates.
3+
* All Rights reserved.
4+
* $COPYRIGHT$
5+
*
6+
* Additional copyrights may follow
7+
*
8+
* $HEADER$
9+
*/
10+
11+
#include "opal_config.h"
12+
13+
#include <assert.h>
14+
15+
#include "opal/class/opal_cstring.h"
16+
#include <stdio.h>
17+
#include <stdlib.h>
18+
19+
#define CHECK_INT(actual, expected) \
20+
if (expected != actual) { \
21+
printf("%s:%d: expected %d, got %d\n", \
22+
__FILE__, __LINE__, expected, actual); \
23+
exit(1); \
24+
}
25+
26+
#define CHECK_BOOL(actual, expected) \
27+
if (expected != actual) { \
28+
printf("%s:%d: expected %s, got %s\n", \
29+
__FILE__, __LINE__, \
30+
expected ? "true" : "false", \
31+
actual ? "true" : "false"); \
32+
exit(1); \
33+
}
34+
35+
int main(int argc, char *argv[])
36+
{
37+
int ret, int_val;
38+
bool bool_val;
39+
opal_cstring_t *cstr;
40+
41+
/*
42+
* bool tests
43+
*/
44+
cstr = opal_cstring_create("true");
45+
ret = opal_cstring_to_bool(cstr, &bool_val);
46+
CHECK_INT(ret, OPAL_SUCCESS);
47+
CHECK_BOOL(bool_val, true);
48+
OBJ_RELEASE(cstr);
49+
50+
cstr = opal_cstring_create("yes");
51+
ret = opal_cstring_to_bool(cstr, &bool_val);
52+
CHECK_INT(ret, OPAL_SUCCESS);
53+
CHECK_BOOL(bool_val, true);
54+
OBJ_RELEASE(cstr);
55+
56+
cstr = opal_cstring_create("false");
57+
ret = opal_cstring_to_bool(cstr, &bool_val);
58+
CHECK_INT(ret, OPAL_SUCCESS);
59+
CHECK_BOOL(bool_val, false);
60+
OBJ_RELEASE(cstr);
61+
62+
cstr = opal_cstring_create("no");
63+
ret = opal_cstring_to_bool(cstr, &bool_val);
64+
CHECK_INT(ret, OPAL_SUCCESS);
65+
CHECK_BOOL(bool_val, false);
66+
OBJ_RELEASE(cstr);
67+
68+
cstr = opal_cstring_create("1");
69+
ret = opal_cstring_to_bool(cstr, &bool_val);
70+
CHECK_INT(ret, OPAL_SUCCESS);
71+
CHECK_BOOL(bool_val, true);
72+
OBJ_RELEASE(cstr);
73+
74+
cstr = opal_cstring_create("0");
75+
ret = opal_cstring_to_bool(cstr, &bool_val);
76+
CHECK_INT(ret, OPAL_SUCCESS);
77+
CHECK_BOOL(bool_val, false);
78+
OBJ_RELEASE(cstr);
79+
80+
cstr = opal_cstring_create("1.0");
81+
ret = opal_cstring_to_bool(cstr, &bool_val);
82+
CHECK_INT(ret, OPAL_SUCCESS);
83+
CHECK_BOOL(bool_val, true);
84+
OBJ_RELEASE(cstr);
85+
86+
cstr = opal_cstring_create("0.0");
87+
ret = opal_cstring_to_bool(cstr, &bool_val);
88+
CHECK_INT(ret, OPAL_SUCCESS);
89+
CHECK_BOOL(bool_val, false);
90+
OBJ_RELEASE(cstr);
91+
92+
cstr = opal_cstring_create("foobar");
93+
ret = opal_cstring_to_bool(cstr, &bool_val);
94+
CHECK_INT(ret, OPAL_ERR_BAD_PARAM);
95+
OBJ_RELEASE(cstr);
96+
97+
cstr = opal_cstring_create(NULL);
98+
ret = opal_cstring_to_bool(cstr, &bool_val);
99+
CHECK_INT(ret, OPAL_ERR_BAD_PARAM);
100+
OBJ_RELEASE(cstr);
101+
102+
103+
/*
104+
* bool string tests
105+
*/
106+
bool_val = opal_str_to_bool("true");
107+
CHECK_BOOL(bool_val, true);
108+
109+
bool_val = opal_str_to_bool("yes");
110+
CHECK_BOOL(bool_val, true);
111+
112+
bool_val = opal_str_to_bool("false");
113+
CHECK_BOOL(bool_val, false);
114+
115+
bool_val = opal_str_to_bool("no");
116+
CHECK_BOOL(bool_val, false);
117+
118+
bool_val = opal_str_to_bool("1");
119+
CHECK_BOOL(bool_val, true);
120+
121+
bool_val = opal_str_to_bool("0");
122+
CHECK_BOOL(bool_val, false);
123+
124+
bool_val = opal_str_to_bool("1.0");
125+
CHECK_BOOL(bool_val, true);
126+
127+
bool_val = opal_str_to_bool("0.0");
128+
CHECK_BOOL(bool_val, false);
129+
130+
bool_val = opal_str_to_bool("foobar");
131+
CHECK_BOOL(bool_val, false);
132+
133+
bool_val = opal_str_to_bool(NULL);
134+
CHECK_BOOL(bool_val, false);
135+
136+
/*
137+
* integer tests
138+
*/
139+
cstr = opal_cstring_create("1");
140+
ret = opal_cstring_to_int(cstr, &int_val);
141+
CHECK_INT(ret, OPAL_SUCCESS);
142+
CHECK_INT(int_val, 1);
143+
OBJ_RELEASE(cstr);
144+
145+
cstr = opal_cstring_create("0");
146+
ret = opal_cstring_to_int(cstr, &int_val);
147+
CHECK_INT(ret, OPAL_SUCCESS);
148+
CHECK_INT(int_val, 0);
149+
OBJ_RELEASE(cstr);
150+
151+
/* test intentional overflow cases */
152+
if (sizeof(long) > sizeof(int)) {
153+
long input = (long)INT_MAX + 1L;
154+
char *input_str;
155+
156+
ret = asprintf(&input_str, "%ld", input);
157+
if (ret < 0) {
158+
printf("%s:%d: asprintf()", __FILE__, __LINE__);
159+
exit(1);
160+
}
161+
162+
cstr = opal_cstring_create(input_str);
163+
ret = opal_cstring_to_int(cstr, &int_val);
164+
CHECK_INT(ret, OPAL_ERR_BAD_PARAM);
165+
OBJ_RELEASE(cstr);
166+
free(input_str);
167+
}
168+
169+
/* yes, this may be redundant from above, but always try both
170+
cases for simplicity */
171+
if (sizeof(long long) > sizeof(int)) {
172+
long long input = (long long)INT_MAX + 1L;
173+
char *input_str;
174+
175+
ret = asprintf(&input_str, "%lld", input);
176+
if (ret < 0) {
177+
printf("%s:%d: asprintf()", __FILE__, __LINE__);
178+
exit(1);
179+
}
180+
181+
cstr = opal_cstring_create(input_str);
182+
ret = opal_cstring_to_int(cstr, &int_val);
183+
CHECK_INT(ret, OPAL_ERR_BAD_PARAM);
184+
OBJ_RELEASE(cstr);
185+
free(input_str);
186+
}
187+
188+
{
189+
long input = (long)LONG_MAX;
190+
char *input_str;
191+
192+
ret = asprintf(&input_str, "%ld0", input);
193+
if (ret < 0) {
194+
printf("%s:%d: asprintf()", __FILE__, __LINE__);
195+
exit(1);
196+
}
197+
198+
cstr = opal_cstring_create(input_str);
199+
ret = opal_cstring_to_int(cstr, &int_val);
200+
CHECK_INT(ret, OPAL_ERR_BAD_PARAM);
201+
OBJ_RELEASE(cstr);
202+
free(input_str);
203+
}
204+
205+
206+
cstr = opal_cstring_create("1.0");
207+
ret = opal_cstring_to_int(cstr, &int_val);
208+
CHECK_INT(ret, OPAL_ERR_BAD_PARAM);
209+
OBJ_RELEASE(cstr);
210+
211+
cstr = opal_cstring_create("0.0");
212+
ret = opal_cstring_to_int(cstr, &int_val);
213+
CHECK_INT(ret, OPAL_ERR_BAD_PARAM);
214+
OBJ_RELEASE(cstr);
215+
216+
cstr = opal_cstring_create("foobar");
217+
ret = opal_cstring_to_int(cstr, &int_val);
218+
CHECK_INT(ret, OPAL_ERR_BAD_PARAM);
219+
OBJ_RELEASE(cstr);
220+
221+
cstr = opal_cstring_create(NULL);
222+
ret = opal_cstring_to_int(cstr, &int_val);
223+
CHECK_INT(ret, OPAL_ERR_BAD_PARAM);
224+
OBJ_RELEASE(cstr);
225+
226+
cstr = opal_cstring_create("true");
227+
ret = opal_cstring_to_int(cstr, &int_val);
228+
CHECK_INT(ret, OPAL_ERR_BAD_PARAM);
229+
OBJ_RELEASE(cstr);
230+
231+
cstr = opal_cstring_create("yes");
232+
ret = opal_cstring_to_int(cstr, &int_val);
233+
CHECK_INT(ret, OPAL_ERR_BAD_PARAM);
234+
OBJ_RELEASE(cstr);
235+
236+
cstr = opal_cstring_create("false");
237+
ret = opal_cstring_to_int(cstr, &int_val);
238+
CHECK_INT(ret, OPAL_ERR_BAD_PARAM);
239+
OBJ_RELEASE(cstr);
240+
241+
cstr = opal_cstring_create("no");
242+
ret = opal_cstring_to_int(cstr, &int_val);
243+
CHECK_INT(ret, OPAL_ERR_BAD_PARAM);
244+
OBJ_RELEASE(cstr);
245+
246+
return 0;
247+
}

0 commit comments

Comments
 (0)