Bug #633
closedutils_parse_size_suffix suffers from several problems
100%
Description
The lttng-tools/src/common/utils.c
utils_parse_size_suffix
function is used by the snapshot
command to handle its max-size
option and the enable-channel
command to handle its subbuf-size
and tracefile-size
options. As currently written, it suffers from several problems.
- It accepts a leading 0 (without a following x or X) to represent an octal value, but simultaneously accepts the digits 8 and 9 in said octal value (this yields –1).
- It does not support the leading 0X hexadecimal indicator but does support a leading 0x (see following).
- It accepts a leading 0x to represent an hexadecimal value but rejects the a..f and A..F digits in said value.
The function uses strtoull()
; for that function's string argument specification, see strtol()
at http://www.cplusplus.com/reference/cstdlib/strtol/
int utils_parse_size_suffix(char *str, uint64_t *size) { [...] /* Compile regex */ ret = regcomp(®ex, "^\\(0x\\)\\{0,1\\}[0-9][0-9]*\\([kKMG]\\{0,1\\}\\)$", 0); [...] base_size = strtoull(str, NULL, 0); if (errno != 0) { [...] }
I think the regex should rather be:
ret = regcomp(®ex, "^(((0x|0X)[0-9A-Fa-f]+)|(0[0-7]*)|([1-9][0-9]*))[kKMG]?$", REG_EXTENDED);
(Someone needs to check my syntax) (thanks to http://www.greenend.org.uk/rjk/tech/regexp.html)
Alternately, the range of strings utils_parse_size_suffix
accepts could be restricted (to drop octal support, for instance).
Updated by Simon Marchi about 11 years ago
Good points. Your regex is very easy to understand (hex|oct|dec). I suggest you submit a patch and augment the test cases in tests/unit/test_utils_parse_size_suffix.c which are clearly lacking, since they didn't catch these bugs.
Updated by David Goulet almost 11 years ago
- Status changed from Confirmed to Feedback
Is it possible for you to submit a patch with that fixes and also, like Simon suggested, improve the unit test?
Updated by Simon Marchi over 10 years ago
- Status changed from Feedback to Resolved
- % Done changed from 0 to 100
Applied in changeset 5983a922b5e591a0fd90800e482e1ab8b89a4281.