There are a number of cases in the configuration where one needs to set a size (think buffer sizes), and counting zeroes is extremely annoying and unintuitive. For this reason, we introduce number unit suffixes for kilo-, mega- and giga-: K, M, G, respectively (case-insensitive, optionally followed by a 'b' or 'B', for byte). The parser also understands KiB, MiB, etc to indicate base-two units. Anywhere where you can specify a number, you'll be able to specify one with a unit suffix from now on: log-fifo-size(200M) A function to parse longs already existed in basic-funcs (tf_parse_int), which was moved to lib/parse-number.c now, and enhanced to recognise the unit suffixes too. This also closes #28. Signed-off-by: Balazs Scheidler <bazsi@balabit.hu> Signed-off-by: Gergely Nagy <algernon@balabit.hu> --- lib/Makefile.am | 2 + lib/cfg-lex.l | 10 ++- lib/parse-number.c | 161 ++++++++++++++++++++++++++++++++++++ lib/parse-number.h | 9 ++ lib/tests/Makefile.am | 8 +- lib/tests/test_parse_number.c | 118 ++++++++++++++++++++++++++ modules/basicfuncs/basic-funcs.c | 21 +---- modules/basicfuncs/numeric-funcs.c | 4 +- modules/basicfuncs/str-funcs.c | 4 +- 9 files changed, 310 insertions(+), 27 deletions(-) create mode 100644 lib/parse-number.c create mode 100644 lib/parse-number.h create mode 100644 lib/tests/test_parse_number.c diff --git a/lib/Makefile.am b/lib/Makefile.am index f7eb421..5814596 100644 --- a/lib/Makefile.am +++ b/lib/Makefile.am @@ -63,6 +63,7 @@ pkginclude_HEADERS += \ lib/ml-batched-timer.h \ lib/msg-format.h \ lib/nvtable.h \ + lib/parse-number.h \ lib/persist-state.h \ lib/plugin.h \ lib/plugin-types.h \ @@ -139,6 +140,7 @@ lib_libsyslog_ng_la_SOURCES = \ lib/ml-batched-timer.c \ lib/msg-format.c \ lib/nvtable.c \ + lib/parse-number.c \ lib/persist-state.c \ lib/plugin.c \ lib/poll-events.c \ diff --git a/lib/cfg-lex.l b/lib/cfg-lex.l index 7b1a81d..b923a7c 100644 --- a/lib/cfg-lex.l +++ b/lib/cfg-lex.l @@ -27,7 +27,7 @@ #include "cfg-lexer.h" #include "cfg-grammar.h" #include "messages.h" -#include "misc.h" +#include "parse-number.h" #include <string.h> #include <strings.h> @@ -160,7 +160,13 @@ word [^ \#'"\(\)\{\}\\;\n\t,|\.@:] (-|\+)?{digit}+\.{digit}+ { yylval->fnum = strtod(yytext, NULL); return LL_FLOAT; } 0x{xdigit}+ { yylval->num = strtoll(yytext + 2, NULL, 16); return LL_NUMBER; } 0{odigit}+ { yylval->num = strtoll(yytext + 1, NULL, 8); return LL_NUMBER; } -(-|\+)?{digit}+ { yylval->num = strtoll(yytext, NULL, 10); return LL_NUMBER; } +(-|\+)?{digit}+(M|m|G|g|k|K)?(i|I)?(b|B)? { + if (!parse_number_with_suffix(yytext, &yylval->num)) + { + YY_FATAL_ERROR("Invalid number specification"); + } + return LL_NUMBER; + } ({word}+(\.)?)*{word}+ { return cfg_lexer_lookup_keyword(yyextra, yylval, yylloc, yytext); } \( | \) | diff --git a/lib/parse-number.c b/lib/parse-number.c new file mode 100644 index 0000000..b9b1be5 --- /dev/null +++ b/lib/parse-number.c @@ -0,0 +1,161 @@ +#include "parse-number.h" + +#include <string.h> +#include <errno.h> +#include <stdlib.h> + +static gboolean +_valid_unit(const gchar unit_char) +{ + return (unit_char == 'B' || unit_char == 'b'); +} + +static gboolean +_valid_exponent(const gchar exponent_char) +{ + gchar e = exponent_char; + + return e == 'k' || e == 'K' || + e == 'm' || e == 'M' || + e == 'g' || e == 'G'; +} + +static gboolean +_parse_suffix(const gchar *suffix, gchar *exponent_char, gchar *base_char, gchar *unit_char) +{ + gint suffix_len; + + suffix_len = strlen(suffix); + if (suffix_len > 3) + return FALSE; + if (suffix_len == 0) + return TRUE; + + if (suffix_len == 3) + { + *exponent_char = suffix[0]; + *base_char = suffix[1]; + *unit_char = suffix[2]; + } + else if (suffix_len == 2) + { + *exponent_char = suffix[0]; + if (_valid_unit(suffix[1])) + *unit_char = suffix[1]; + else + *base_char = suffix[1]; + } + else if (suffix_len == 1) + { + if (_valid_exponent(suffix[0])) + *exponent_char = suffix[0]; + else if (_valid_unit(suffix[0])) + *unit_char = suffix[0]; + else + return FALSE; + } + return TRUE; +} + +static gboolean +_determine_multiplier(gchar base_char, glong *multiplier) +{ + if (base_char == 0) + *multiplier = 1000; + else if (base_char == 'I' || base_char == 'i') + *multiplier = 1024; + else + return FALSE; + return TRUE; +} + +static gboolean +_validate_unit(gchar unit_char) +{ + if (unit_char && !_valid_unit(unit_char)) + return FALSE; + return TRUE; +} + +static gboolean +_process_exponent(gchar exponent_char, glong *d, glong multiplier) +{ + switch (exponent_char) + { + case 'G': + case 'g': + (*d) *= multiplier; + case 'm': + case 'M': + (*d) *= multiplier; + case 'K': + case 'k': + (*d) *= multiplier; + case 0: + return TRUE; + default: + return FALSE; + } +} + +static gboolean +_process_suffix(const gchar *suffix, glong *d) +{ + gchar exponent_char = 0, base_char = 0, unit_char = 0; + glong multiplier = 0; + + if (!_parse_suffix(suffix, &exponent_char, &base_char, &unit_char)) + return FALSE; + + if (!_determine_multiplier(base_char, &multiplier)) + return FALSE; + + if (!_validate_unit(unit_char)) + return FALSE; + + if (!_process_exponent(exponent_char, d, multiplier)) + return FALSE; + + return TRUE; +} + +static gboolean +_parse_number(const gchar *s, gchar **endptr, long *d) +{ + glong val; + + errno = 0; + val = strtoll(s, endptr, 10); + + if ((errno == ERANGE && (val == LONG_MAX || val == LONG_MIN)) + || (errno != 0 && val == 0)) + return FALSE; + + if (*endptr == s) + return FALSE; + + *d = val; + return TRUE; +} + +gboolean +parse_number(const gchar *s, glong *d) +{ + gchar *endptr; + + if (!_parse_number(s, &endptr, d)) + return FALSE; + if (*endptr) + return FALSE; + return TRUE; +} + +gboolean +parse_number_with_suffix(const gchar *s, glong *d) +{ + gchar *endptr; + + if (!_parse_number(s, &endptr, d)) + return FALSE; + return _process_suffix(endptr, d); +} diff --git a/lib/parse-number.h b/lib/parse-number.h new file mode 100644 index 0000000..e8e0c52 --- /dev/null +++ b/lib/parse-number.h @@ -0,0 +1,9 @@ +#ifndef PARSE_NUMBER_H_INCLUDED +#define PARSE_NUMBER_H_INCLUDED + +#include "syslog-ng.h" + +gboolean parse_number_with_suffix(const gchar *str, glong *result); +gboolean parse_number(const gchar *str, glong *result); + +#endif diff --git a/lib/tests/Makefile.am b/lib/tests/Makefile.am index 6bead1d..ad5cd26 100644 --- a/lib/tests/Makefile.am +++ b/lib/tests/Makefile.am @@ -1,7 +1,8 @@ lib_tests_TESTS = \ lib/tests/test_log_message \ lib/tests/test_cfg_lexer_subst \ - lib/tests/test_type_hints + lib/tests/test_type_hints \ + lib/tests/test_parse_number check_PROGRAMS += ${lib_tests_TESTS} @@ -19,3 +20,8 @@ lib_tests_test_type_hints_CFLAGS = \ $(TEST_CFLAGS) lib_tests_test_type_hints_LDADD = \ $(TEST_LDADD) + +lib_tests_test_parse_number_CFLAGS = \ + $(TEST_CFLAGS) +lib_tests_test_parse_number_LDADD = \ + $(TEST_LDADD) diff --git a/lib/tests/test_parse_number.c b/lib/tests/test_parse_number.c new file mode 100644 index 0000000..5895005 --- /dev/null +++ b/lib/tests/test_parse_number.c @@ -0,0 +1,118 @@ +#include <stdlib.h> + +#include "testutils.h" +#include "parse-number.h" + +static void +assert_parse_with_suffix(const gchar *str, long expected) +{ + long n; + gboolean res; + + res = parse_number_with_suffix(str, &n); + + assert_gboolean(res, TRUE, "Parsing (w/ suffix) %s failed", str); + assert_gint64(n, expected, "Parsing (w/ suffix) %s failed", str); +} + +static void +assert_parse_with_suffix_fails(const gchar *str) +{ + long n; + gboolean res; + + res = parse_number_with_suffix(str, &n); + assert_gboolean(res, FALSE, "Parsing (w/ suffix) %s succeeded, while expecting failure", str); +} + +static void +assert_parse(const gchar *str, long expected) +{ + long n; + gboolean res; + + res = parse_number(str, &n); + + assert_gboolean(res, TRUE, "Parsing (w/o suffix) %s failed", str); + assert_gint64(n, expected, "Parsing (w/o suffix) %s failed", str); +} + +static void +assert_parse_fails(const gchar *str) +{ + long n; + gboolean res; + + res = parse_number(str, &n); + + assert_gboolean(res, FALSE, "Parsing (w/o suffix) %s succeeded, while expecting failure", str); +} + +static void +test_simple_numbers_are_parsed_properly(void) +{ + assert_parse_with_suffix("1234", 1234); + assert_parse_with_suffix("+1234", 1234); + assert_parse_with_suffix("-1234", -1234); + + assert_parse("1234", 1234); +} + +static void +test_exponent_suffix_is_parsed_properly(void) +{ + assert_parse_with_suffix("1K", 1000); + assert_parse_with_suffix("1k", 1000); + assert_parse_with_suffix("1m", 1000 * 1000); + assert_parse_with_suffix("1M", 1000 * 1000); + assert_parse_with_suffix("1G", 1000 * 1000 * 1000); + assert_parse_with_suffix("1g", 1000 * 1000 * 1000); +} + +static void +test_byte_units_are_accepted(void) +{ + assert_parse_with_suffix("1b", 1); + assert_parse_with_suffix("1B", 1); + assert_parse_with_suffix("1Kb", 1000); + assert_parse_with_suffix("1kB", 1000); + assert_parse_with_suffix("1mb", 1000 * 1000); + assert_parse_with_suffix("1MB", 1000 * 1000); + assert_parse_with_suffix("1Gb", 1000 * 1000 * 1000); + assert_parse_with_suffix("1gB", 1000 * 1000 * 1000); +} + +static void +test_base2_is_selected_by_an_i_modifier(void) +{ + assert_parse_with_suffix("1Kib", 1024); + assert_parse_with_suffix("1kiB", 1024); + assert_parse_with_suffix("1Ki", 1024); + assert_parse_with_suffix("1kI", 1024); + assert_parse_with_suffix("1mib", 1024 * 1024); + assert_parse_with_suffix("1MiB", 1024 * 1024); + assert_parse_with_suffix("1Gib", 1024 * 1024 * 1024); + assert_parse_with_suffix("1giB", 1024 * 1024 * 1024); +} + +static void +test_invalid_formats_are_not_accepted(void) +{ + assert_parse_with_suffix_fails("1234Z"); + assert_parse_with_suffix_fails("1234kZ"); + assert_parse_with_suffix_fails("1234kdZ"); + assert_parse_with_suffix_fails("1234kiZ"); + assert_parse_fails("1234kiZ"); +} + +int +main(int argc, char *argv[]) +{ + test_simple_numbers_are_parsed_properly(); + test_exponent_suffix_is_parsed_properly(); + test_byte_units_are_accepted(); + test_base2_is_selected_by_an_i_modifier(); + test_invalid_formats_are_not_accepted(); + + return 0; +} diff --git a/modules/basicfuncs/basic-funcs.c b/modules/basicfuncs/basic-funcs.c index cb73b02..523f6cf 100644 --- a/modules/basicfuncs/basic-funcs.c +++ b/modules/basicfuncs/basic-funcs.c @@ -26,6 +26,7 @@ #include "filter/filter-expr.h" #include "filter/filter-expr-parser.h" #include "cfg.h" +#include "parse-number.h" #include "str-format.h" #include "plugin-types.h" @@ -33,26 +34,6 @@ #include <errno.h> #include <string.h> -static gboolean -tf_parse_int(const gchar *s, long *d) -{ - gchar *endptr; - glong val; - - errno = 0; - val = strtoll(s, &endptr, 10); - - if ((errno == ERANGE && (val == LONG_MAX || val == LONG_MIN)) - || (errno != 0 && val == 0)) - return FALSE; - - if (endptr == s || *endptr != '\0') - return FALSE; - - *d = val; - return TRUE; -} - /* in order to avoid having to declare all construct functions, we * include them all here. If it causes compilation times to increase * drastically, we should probably make them into separate compilation diff --git a/modules/basicfuncs/numeric-funcs.c b/modules/basicfuncs/numeric-funcs.c index 6bd00ef..9125c3c 100644 --- a/modules/basicfuncs/numeric-funcs.c +++ b/modules/basicfuncs/numeric-funcs.c @@ -32,7 +32,7 @@ tf_num_parse(gint argc, GString *argv[], return FALSE; } - if (!tf_parse_int(argv[0]->str, n)) + if (!parse_number_with_suffix(argv[0]->str, n)) { msg_debug("Parsing failed, template function's first argument is not a number", evt_tag_str("function", func_name), @@ -40,7 +40,7 @@ tf_num_parse(gint argc, GString *argv[], return FALSE; } - if (!tf_parse_int(argv[1]->str, m)) + if (!parse_number_with_suffix(argv[1]->str, m)) { msg_debug("Parsing failed, template function's first argument is not a number", evt_tag_str("function", func_name), diff --git a/modules/basicfuncs/str-funcs.c b/modules/basicfuncs/str-funcs.c index 6c8ae3c..060795d 100644 --- a/modules/basicfuncs/str-funcs.c +++ b/modules/basicfuncs/str-funcs.c @@ -79,14 +79,14 @@ tf_substr(LogMessage *msg, gint argc, GString *argv[], GString *result) return; /* get offset position from second argument */ - if (!tf_parse_int(argv[1]->str, &start)) { + if (!parse_number(argv[1]->str, &start)) { msg_error("$(substr) parsing failed, start could not be parsed", evt_tag_str("start", argv[1]->str), NULL); return; } /* if we were called with >2 arguments, third was desired length */ if (argc > 2) { - if (!tf_parse_int(argv[2]->str, &len)) { + if (!parse_number(argv[2]->str, &len)) { msg_error("$(substr) parsing failed, length could not be parsed", evt_tag_str("length", argv[2]->str), NULL); return; } -- 1.7.10.4