From 5983a922b5e591a0fd90800e482e1ab8b89a4281 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Thu, 10 Apr 2014 11:30:19 -0400 Subject: [PATCH] Fix: rework utils_parse_size_suffix Ok, so there are a lot of problems with this function (sorry :|). Taking the regex road is probably to complicated for nothing, so here is a version without regexes. I added many test cases as suggested by Sandeep Chaudhary and Daniel Thibault. I tested on both Intel 32 and 64 bits. Fixes #633 Signed-off-by: Simon Marchi Signed-off-by: David Goulet --- src/common/utils.c | 132 +++++++++------------- src/common/utils.h | 2 +- tests/unit/test_utils_parse_size_suffix.c | 54 ++++++++- 3 files changed, 107 insertions(+), 81 deletions(-) diff --git a/src/common/utils.c b/src/common/utils.c index 31596ddd3..815965bc4 100644 --- a/src/common/utils.c +++ b/src/common/utils.c @@ -28,7 +28,6 @@ #include #include #include -#include #include #include @@ -650,42 +649,10 @@ error: return ret; } -/** - * Prints the error message corresponding to a regex error code. - * - * @param errcode The error code. - * @param regex The regex object that produced the error code. - */ -static void regex_print_error(int errcode, regex_t *regex) -{ - /* Get length of error message and allocate accordingly */ - size_t length; - char *buffer; - - assert(regex != NULL); - - length = regerror(errcode, regex, NULL, 0); - if (length == 0) { - ERR("regerror returned a length of 0"); - return; - } - - buffer = zmalloc(length); - if (!buffer) { - ERR("regex_print_error: zmalloc failed"); - return; - } - - /* Get and print error message */ - regerror(errcode, regex, buffer, length); - ERR("regex error: %s\n", buffer); - free(buffer); - -} /** * Parse a string that represents a size in human readable format. It - * supports decimal integers suffixed by 'k', 'M' or 'G'. + * supports decimal integers suffixed by 'k', 'K', 'M' or 'G'. * * The suffix multiply the integer by: * 'k': 1024 @@ -693,83 +660,90 @@ static void regex_print_error(int errcode, regex_t *regex) * 'G': 1024^3 * * @param str The string to parse. - * @param size Pointer to a size_t that will be filled with the + * @param size Pointer to a uint64_t that will be filled with the * resulting size. * * @return 0 on success, -1 on failure. */ LTTNG_HIDDEN -int utils_parse_size_suffix(char *str, uint64_t *size) +int utils_parse_size_suffix(const char * const str, uint64_t * const size) { - regex_t regex; int ret; - const int nmatch = 3; - regmatch_t suffix_match, matches[nmatch]; - unsigned long long base_size; + uint64_t base_size; long shift = 0; + const char *str_end; + char *num_end; if (!str) { - return 0; - } - - /* Compile regex */ - ret = regcomp(®ex, "^\\(0x\\)\\{0,1\\}[0-9][0-9]*\\([kKMG]\\{0,1\\}\\)$", 0); - if (ret != 0) { - regex_print_error(ret, ®ex); + DBG("utils_parse_size_suffix: received a NULL string."); ret = -1; goto end; } - /* Match regex */ - ret = regexec(®ex, str, nmatch, matches, 0); - if (ret != 0) { + /* strtoull will accept a negative number, but we don't want to. */ + if (strchr(str, '-') != NULL) { + DBG("utils_parse_size_suffix: invalid size string, should not contain '-'."); ret = -1; - goto free; + goto end; } - /* There is a match ! */ + /* str_end will point to the \0 */ + str_end = str + strlen(str); errno = 0; - base_size = strtoull(str, NULL, 0); + base_size = strtoull(str, &num_end, 0); if (errno != 0) { - PERROR("strtoull"); + PERROR("utils_parse_size_suffix strtoull"); ret = -1; - goto free; + goto end; } - /* Check if there is a suffix */ - suffix_match = matches[2]; - if (suffix_match.rm_eo - suffix_match.rm_so == 1) { - switch (*(str + suffix_match.rm_so)) { - case 'K': - case 'k': - shift = KIBI_LOG2; - break; - case 'M': - shift = MEBI_LOG2; - break; - case 'G': - shift = GIBI_LOG2; - break; - default: - ERR("parse_human_size: invalid suffix"); - ret = -1; - goto free; - } + if (num_end == str) { + /* strtoull parsed nothing, not good. */ + DBG("utils_parse_size_suffix: strtoull had nothing good to parse."); + ret = -1; + goto end; + } + + /* Check if a prefix is present. */ + switch (*num_end) { + case 'G': + shift = GIBI_LOG2; + num_end++; + break; + case 'M': /* */ + shift = MEBI_LOG2; + num_end++; + break; + case 'K': + case 'k': + shift = KIBI_LOG2; + num_end++; + break; + case '\0': + break; + default: + DBG("utils_parse_size_suffix: invalid suffix."); + ret = -1; + goto end; + } + + /* Check for garbage after the valid input. */ + if (num_end != str_end) { + DBG("utils_parse_size_suffix: Garbage after size string."); + ret = -1; + goto end; } *size = base_size << shift; /* Check for overflow */ if ((*size >> shift) != base_size) { - ERR("parse_size_suffix: oops, overflow detected."); + DBG("utils_parse_size_suffix: oops, overflow detected."); ret = -1; - goto free; + goto end; } ret = 0; - -free: - regfree(®ex); end: return ret; } diff --git a/src/common/utils.h b/src/common/utils.h index 63290a7bd..b872b5319 100644 --- a/src/common/utils.h +++ b/src/common/utils.h @@ -43,7 +43,7 @@ int utils_create_stream_file(const char *path_name, char *file_name, uint64_t si int utils_rotate_stream_file(char *path_name, char *file_name, uint64_t size, uint64_t count, int uid, int gid, int out_fd, uint64_t *new_count, int *stream_fd); -int utils_parse_size_suffix(char *str, uint64_t *size); +int utils_parse_size_suffix(char const * const str, uint64_t * const size); int utils_get_count_order_u32(uint32_t x); char *utils_get_home_dir(void); char *utils_get_user_home_dir(uid_t uid); diff --git a/tests/unit/test_utils_parse_size_suffix.c b/tests/unit/test_utils_parse_size_suffix.c index 990aa1b84..45a87b06c 100644 --- a/tests/unit/test_utils_parse_size_suffix.c +++ b/tests/unit/test_utils_parse_size_suffix.c @@ -43,11 +43,63 @@ static struct valid_test_input valid_tests_inputs[] = { { "0x1234k", 4771840 }, { "32M", 33554432 }, { "1024G", 1099511627776 }, + { "0X400", 1024 }, + { "0x40a", 1034 }, + { "0X40b", 1035 }, + { "0x40C", 1036 }, + { "0X40D", 1037 }, + { "0x40e", 1038 }, + { "0X40f", 1039 }, + { "00", 0 }, + { "0k", 0 }, + { "0K", 0 }, + { "0M", 0 }, + { "0G", 0 }, + { "00k", 0 }, + { "00K", 0 }, + { "00M", 0 }, + { "00G", 0 }, + { "0x0", 0 }, + { "0X0", 0 }, + { "0x0k", 0 }, + { "0X0K", 0 }, + { "0x0M", 0 }, + { "0X0G", 0 }, + { "0X40G", 68719476736 }, + { "0300k", 196608 }, + { "0300K", 196608 }, + { "030M", 25165824 }, + { "020G", 17179869184 }, + { "0xa0k", 163840 }, + { "0xa0K", 163840 }, + { "0XA0M", 167772160 }, + { "0xA0G", 171798691840 }, }; static const int num_valid_tests = sizeof(valid_tests_inputs) / sizeof(valid_tests_inputs[0]); /* Invalid test cases */ -static char *invalid_tests_inputs[] = { "", "-1", "k", "4611686018427387904G" }; +static char *invalid_tests_inputs[] = { + "", + " ", + "-1", + "k", + "4611686018427387904G", + "0x40g", + "08", + "09", + "0x", + "x0", + "0xx0", + "07kK", + "0xk", + "0XM", + "0xG", + "0x0MM", + "0X0GG", + "0a", + "0B", +}; + static const int num_invalid_tests = sizeof(invalid_tests_inputs) / sizeof(invalid_tests_inputs[0]); static void test_utils_parse_size_suffix(void) -- 2.34.1