[lttng-dev] [PATCH lttng-tools] utils: Rework utils_parse_size_suffix
Simon Marchi
simon.marchi at polymtl.ca
Thu Apr 10 11:30:19 EDT 2014
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.
Would fix #633
Signed-off-by: Simon Marchi <simon.marchi at polymtl.ca>
---
src/common/utils.c | 136 +++++++++++++-----------------
src/common/utils.h | 2 +-
tests/unit/test_utils_parse_size_suffix.c | 54 +++++++++++-
3 files changed, 111 insertions(+), 81 deletions(-)
diff --git a/src/common/utils.c b/src/common/utils.c
index 31596dd..9326080 100644
--- a/src/common/utils.c
+++ b/src/common/utils.c
@@ -28,7 +28,6 @@
#include <sys/types.h>
#include <unistd.h>
#include <inttypes.h>
-#include <regex.h>
#include <grp.h>
#include <pwd.h>
@@ -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,94 @@ 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.\n");
+ 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.\n");
+ 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 63290a7..b872b53 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 990aa1b..45a87b0 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)
--
1.9.1
More information about the lttng-dev
mailing list