d/numam-spdk
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

string: spdk_strtol to delegate additional error checking

Browse Source 
Error check of strtol is left to users of it. But some use cases
of strtol in SPDK do not have enough error check yet.

For example, strtol returns 0 if there were no digits at all.

It should be avoided for each use case to add enough error checking
for strtol.

Hence spdk_strtol and spdk_strtoll do additional error checking
according to the description of manual of strtol.

Besides, there is no use case of negative number now, and to keep
simplicity, spdk_trtol and spdk_strtoll allows only strings that
is positive number or zero.

As a result of this policy, callers of them only have to check if
the return value is not negative.

Subsequent patches will replace atoi to spdk_strtol because atoi
does not have error check.

Change-Id: If3d549970595e53b1141674e47710fe4dd062bc5
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/c/441626
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: wuzhouhui <wuzhouhui@kingsoft.com>
Reviewed-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Chandler-Test-Pool: SPDK Automated Test System <sys_sgsw@intel.com>
This commit is contained in:
Shuhei Matsumoto 2019-01-23 08:27:37 +09:00 committed by Jim Harris
parent b5c7ae072d
commit b78e763c1a
8 changed files with 245 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
include/spdk/string.h
View File

@ -236,6 +236,34 @@ int spdk_parse_capacity(const char *cap_str, uint64_t *cap, bool *has_prefix);
*/
bool spdk_mem_all_zero(const void *data, size_t size);
/**
* Convert the string in nptr to a long integer value according to the given base.
*
* spdk_strtol() does the additional error checking and allows only strings that
* contains only numbers and is positive number or zero. The caller only has to check
* if the return value is not negative.
*
* \param nptr String containing numbers.
* \param base Base which must be between 2 and 32 inclusive, or be the special value 0.
*
* \return positive number or zero on success, or negative errno on failure.
*/
long int spdk_strtol(const char *nptr, int base);
/**
* Convert the string in nptr to a long long integer value according to the given base.
*
* spdk_strtoll() does the additional error checking and allows only strings that
* contains only numbers and is positive number or zero. The caller only has to check
* if the return value is not negative.
*
* \param nptr String containing numbers.
* \param base Base which must be between 2 and 32 inclusive, or be the special value 0.
*
* \return positive number or zero on success, or negative errno on failure.
*/
long long int spdk_strtoll(const char *nptr, int base);
#ifdef __cplusplus
}
#endif

5
lib/bdev/null/bdev_null.c
View File

@ -38,6 +38,7 @@
#include "spdk/env.h"
#include "spdk/thread.h"
#include "spdk/json.h"
#include "spdk/string.h"
#include "spdk/bdev_module.h"
#include "spdk_internal/log.h"
@ -329,8 +330,8 @@ bdev_null_initialize(void)
block_size = 512;
} else {
errno = 0;
block_size = (int)strtol(val, NULL, 10);
if (errno) {
block_size = (int)spdk_strtol(val, 10);
if (block_size <= 0) {
SPDK_ERRLOG("Null entry %d: Invalid block size %s\n", i, val);
continue;
}

6
lib/bdev/nvme/bdev_nvme.c
View File

@ -1332,13 +1332,11 @@ bdev_nvme_library_init(void)
val = spdk_conf_section_get_val(sp, "TimeoutUsec");
if (val != NULL) {
intval = strtoll(val, NULL, 10);
if (intval == LLONG_MIN || intval == LLONG_MAX) {
intval = spdk_strtoll(val, 10);
if (intval < 0) {
SPDK_ERRLOG("Invalid TimeoutUsec value\n");
rc = -1;
goto end;
} else if (intval < 0) {
intval = 0;
}
}

14
lib/bdev/rbd/bdev_rbd.c
View File

@ -786,6 +786,7 @@ bdev_rbd_library_init(void)
const char *pool_name;
const char *rbd_name;
uint32_t block_size;
long int tmp;
struct spdk_conf_section *sp = spdk_conf_find_section(NULL, "Ceph");
@ -823,13 +824,18 @@ bdev_rbd_library_init(void)
if (val == NULL) {
block_size = 512; /* default value */
} else {
block_size = (int)strtol(val, NULL, 10);
if (block_size & 0x1ff) {
SPDK_ERRLOG("current block_size = %d, it should be multiple of 512\n",
block_size);
tmp = spdk_strtol(val, 10);
if (tmp <= 0) {
SPDK_ERRLOG("Invalid block size\n");
rc = -1;
goto end;
} else if (tmp & 0x1ff) {
SPDK_ERRLOG("current block_size = %ld, it should be multiple of 512\n",
tmp);
rc = -1;
goto end;
}
block_size = (uint32_t)tmp;
}
/* TODO(?): user_id and rbd config values */

4
lib/conf/conf.c
View File

@ -419,7 +419,7 @@ spdk_conf_section_get_intval(struct spdk_conf_section *sp, const char *key)
return -1;
}
value = (int)strtol(v, NULL, 10);
value = (int)spdk_strtol(v, 10);
return value;
}
@ -474,7 +474,7 @@ parse_line(struct spdk_conf *cp, char *lp)
for (p = key; *p != '\0' && !isdigit((int) *p); p++)
;
if (*p != '\0') {
num = (int)strtol(p, NULL, 10);
num = (int)spdk_strtol(p, 10);
} else {
num = 0;
}

62
lib/util/string.c
View File

@ -410,3 +410,65 @@ spdk_mem_all_zero(const void *data, size_t size)
return true;
}
long int
spdk_strtol(const char *nptr, int base)
{
long val;
char *endptr;
/* Since strtoll() can legitimately return 0, LONG_MAX, or LONG_MIN
* on both success and failure, the calling program should set errno
* to 0 before the call.
*/
errno = 0;
val = strtol(nptr, &endptr, base);
if (*endptr != '\0') {
/* Non integer character was found. */
return -EINVAL;
} else if (errno == ERANGE && (val == LONG_MAX || val == LONG_MIN)) {
/* Overflow occurred. */
return -ERANGE;
} else if (errno != 0 && val == 0) {
/* Other error occurred. */
return -errno;
} else if (val < 0) {
/* Input string was negative number. */
return -ERANGE;
}
return val;
}
long long int
spdk_strtoll(const char *nptr, int base)
{
long long val;
char *endptr;
/* Since strtoll() can legitimately return 0, LLONG_MAX, or LLONG_MIN
* on both success and failure, the calling program should set errno
* to 0 before the call.
*/
errno = 0;
val = strtoll(nptr, &endptr, base);
if (*endptr != '\0') {
/* Non integer character was found. */
return -EINVAL;
} else if (errno == ERANGE && (val == LLONG_MAX || val == LLONG_MIN)) {
/* Overflow occurred. */
return -ERANGE;
} else if (errno != 0 && val == 0) {
/* Other error occurred. */
return -errno;
} else if (val < 0) {
/* Input string was negative number. */
return -ERANGE;
}
return val;
}

10
test/bdev/bdevperf/bdevperf.c
View File

@ -827,14 +827,16 @@ static int
bdevperf_parse_arg(int ch, char *arg)
{
long long tmp;
char *end;
if (ch == 'w') {
g_workload_type = optarg;
} else {
tmp = strtoll(optarg, &end, 10);
if (tmp <= INT_MIN || tmp >= INT_MAX) {
fprintf(stderr, "-%c out of range. Parse failed\n", ch);
tmp = spdk_strtoll(optarg, 10);
if (tmp < 0) {
fprintf(stderr, "Parse failed for the option %c.\n", ch);
return tmp;
} else if (tmp >= INT_MAX) {
fprintf(stderr, "Parsed option was too large %c.\n", ch);
return -ERANGE;
}

133
test/unit/lib/util/string.c/string_ut.c
View File

@ -248,6 +248,135 @@ test_sprintf_append_realloc(void)
free(str3);
free(str4);
}
static void
test_strtol(void)
{
long int val;
const char *val1 = "no_digits";
/* LLONG_MIN - 1 */
const char *val2 = "-9223372036854775809";
/* LONG_MIN */
const char *val3 = "-9223372036854775808";
/* LONG_MIN + 1 */
const char *val4 = "-9223372036854775807";
/* LONG_MAX - 1 */
const char *val5 = "9223372036854775806";
/* LONG_MAX */
const char *val6 = "9223372036854775807";
/* LONG_MAX + 1 */
const char *val7 = "9223372036854775808";
/* digits + chars */
const char *val8 = "10_is_ten";
/* chars + digits */
const char *val9 = "ten_is_10";
/* all zeroes */
const char *val10 = "00000000";
/* leading minus sign, but not negative */
const char *val11 = "-0";
val = spdk_strtol(val1, 10);
CU_ASSERT(val == -EINVAL);
val = spdk_strtol(val2, 10);
CU_ASSERT(val == -ERANGE);
val = spdk_strtol(val3, 10);
CU_ASSERT(val == -ERANGE);
val = spdk_strtol(val4, 10);
CU_ASSERT(val == -ERANGE);
val = spdk_strtol(val5, 10);
CU_ASSERT(val == LONG_MAX - 1);
val = spdk_strtol(val6, 10);
CU_ASSERT(val == LONG_MAX);
val = spdk_strtol(val7, 10);
CU_ASSERT(val == -ERANGE);
val = spdk_strtol(val8, 10);
CU_ASSERT(val == -EINVAL);
val = spdk_strtol(val9, 10);
CU_ASSERT(val == -EINVAL);
val = spdk_strtol(val10, 10);
CU_ASSERT(val == 0);
/* Invalid base */
val = spdk_strtol(val10, 1);
CU_ASSERT(val == -EINVAL);
val = spdk_strtol(val11, 10);
CU_ASSERT(val == 0);
}
static void
test_strtoll(void)
{
long long int val;
const char *val1 = "no_digits";
/* LLONG_MIN - 1 */
const char *val2 = "-9223372036854775809";
/* LLONG_MIN */
const char *val3 = "-9223372036854775808";
/* LLONG_MIN + 1 */
const char *val4 = "-9223372036854775807";
/* LLONG_MAX - 1 */
const char *val5 = "9223372036854775806";
/* LLONG_MAX */
const char *val6 = "9223372036854775807";
/* LLONG_MAX + 1 */
const char *val7 = "9223372036854775808";
/* digits + chars */
const char *val8 = "10_is_ten";
/* chars + digits */
const char *val9 = "ten_is_10";
/* all zeroes */
const char *val10 = "00000000";
/* leading minus sign, but not negative */
const char *val11 = "-0";
val = spdk_strtoll(val1, 10);
CU_ASSERT(val == -EINVAL);
val = spdk_strtoll(val2, 10);
CU_ASSERT(val == -ERANGE);
val = spdk_strtoll(val3, 10);
CU_ASSERT(val == -ERANGE);
val = spdk_strtoll(val4, 10);
CU_ASSERT(val == -ERANGE);
val = spdk_strtoll(val5, 10);
CU_ASSERT(val == LLONG_MAX - 1);
val = spdk_strtoll(val6, 10);
CU_ASSERT(val == LLONG_MAX);
val = spdk_strtoll(val7, 10);
CU_ASSERT(val == -ERANGE);
val = spdk_strtoll(val8, 10);
CU_ASSERT(val == -EINVAL);
val = spdk_strtoll(val9, 10);
CU_ASSERT(val == -EINVAL);
val = spdk_strtoll(val10, 10);
CU_ASSERT(val == 0);
/* Invalid base */
val = spdk_strtoll(val10, 1);
CU_ASSERT(val == -EINVAL);
val = spdk_strtoll(val11, 10);
CU_ASSERT(val == 0);
}
int
main(int argc, char **argv)
@ -269,7 +398,9 @@ main(int argc, char **argv)
CU_add_test(suite, "test_parse_ip_addr", test_parse_ip_addr) == NULL ||
CU_add_test(suite, "test_str_chomp", test_str_chomp) == NULL ||
CU_add_test(suite, "test_parse_capacity", test_parse_capacity) == NULL ||
CU_add_test(suite, "test_sprintf_append_realloc", test_sprintf_append_realloc) == NULL) {
CU_add_test(suite, "test_sprintf_append_realloc", test_sprintf_append_realloc) == NULL ||
CU_add_test(suite, "test_strtol", test_strtol) == NULL ||
CU_add_test(suite, "test_strtoll", test_strtoll) == NULL) {
CU_cleanup_registry();
return CU_get_error();
}