Re: [syslog-ng] [PATCH] Add support for number unit suffixes
Hi, I've received your patch via the merge-queue/3.5 branch:
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).
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/misc.c now, and enhanced to recognise the unit suffixes too.
This also closes #28.
Signed-off-by: Gergely Nagy <algernon@balabit.hu>
I really like the change as it definitely seems useful, however I'd have some things I'd like changed, and I've coded this up in a branch named numeric-suffixes. Could you please fold the patches and resubmit them? Thanks. This is the patch I had in mind commit 3fb2cfbbefe5b3d8f2952e807aa40a7b207a7a68 Author: Balazs Scheidler <bazsi@balabit.hu> Date: Tue Jul 30 16:37:31 2013 +0200 parse-number: follow up patch to number-units functionality This patch improves the patch series in a number of ways: - moves the number parsing to its separate module - removes the conflicting rule from the lexer, which would never match - accepts 'i' modifier in suffixes to indicate base2 units - string related functions don't get the suffixes functionality - extends the test suite, and adds test functions to indicate the intent of various asserts - test-num-parse is moved under lib/tests This patch should be folded back into the base set before integration. diff --git a/lib/Makefile.am b/lib/Makefile.am index d16f755..44ee107 100644 --- a/lib/Makefile.am +++ b/lib/Makefile.am @@ -62,6 +62,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 \ @@ -135,6 +136,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/pragma-parser.c \ diff --git a/lib/cfg-lex.l b/lib/cfg-lex.l index ed61c6f..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,14 +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}+(M|m|G|g|k|K)?(b|B)? { - if (!num_parse_int(yytext, &yylval->num)) +(-|\+)?{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; } -(-|\+)?{digit}+ { yylval->num = strtoll(yytext, NULL, 10); return LL_NUMBER; } ({word}+(\.)?)*{word}+ { return cfg_lexer_lookup_keyword(yyextra, yylval, yylloc, yytext); } \( | \) | diff --git a/lib/misc.c b/lib/misc.c index 2d95c78..7a5fad6 100644 --- a/lib/misc.c +++ b/lib/misc.c @@ -635,48 +635,3 @@ utf8_escape_string(const gchar *str, gssize len) return res; } - -gboolean -num_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) - return FALSE; - - switch (*endptr) - { - case 'G': - case 'g': - val *= 1000; - case 'm': - case 'M': - val *= 1000; - case 'K': - case 'k': - val *= 1000; - endptr++; - break; - case '\0': - break; - default: - return FALSE; - } - - if (*endptr == 'b' || *endptr == 'B') - endptr++; - - if (*endptr != '\0') - return FALSE; - - *d = val; - return TRUE; -} 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 7117983..4d88daa 100644 --- a/lib/tests/Makefile.am +++ b/lib/tests/Makefile.am @@ -1,4 +1,8 @@ -lib_tests_TESTS = lib/tests/test_log_message lib/tests/test_cfg_lexer_subst +lib_tests_TESTS = \ + lib/tests/test_log_message \ + lib/tests/test_cfg_lexer_subst \ + lib/tests/test_parse_number + check_PROGRAMS += ${lib_tests_TESTS} lib_tests_test_log_message_CFLAGS = \ @@ -10,3 +14,8 @@ lib_tests_test_cfg_lexer_subst_CFLAGS = \ $(TEST_CFLAGS) lib_tests_test_cfg_lexer_subst_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 c519d86..26fa8bd 100644 --- a/modules/basicfuncs/basic-funcs.c +++ b/modules/basicfuncs/basic-funcs.c @@ -26,7 +26,7 @@ #include "filter/filter-expr.h" #include "filter/filter-expr-parser.h" #include "cfg.h" -#include "misc.h" +#include "parse-number.h" #include "str-format.h" #include "plugin-types.h" diff --git a/modules/basicfuncs/numeric-funcs.c b/modules/basicfuncs/numeric-funcs.c index 39e4d83..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 (!num_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 (!num_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 b0bcdd4..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 (!num_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 (!num_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; } diff --git a/tests/unit/Makefile.am b/tests/unit/Makefile.am index d5c8ac6..8841834 100644 --- a/tests/unit/Makefile.am +++ b/tests/unit/Makefile.am @@ -15,8 +15,7 @@ tests_unit_TESTS = \ tests/unit/test_logwriter \ tests/unit/test_zone \ tests/unit/test_persist_state \ - tests/unit/test_value_pairs \ - tests/unit/test_num_parse + tests/unit/test_value_pairs check_PROGRAMS += \ ${tests_unit_TESTS} @@ -81,10 +80,6 @@ tests_unit_test_persist_state_LDADD = \ tests_unit_test_value_pairs_LDADD = \ $(TEST_LDADD) $(unit_test_extra_modules) -tests_unit_test_num_parse_CFLAGS = ${TEST_CFLAGS} -tests_unit_test_num_parse_LDADD = \ - $(TEST_LDADD) - CLEANFILES += \ test_values.persist \ test_values.persist- diff --git a/tests/unit/test_num_parse.c b/tests/unit/test_num_parse.c deleted file mode 100644 index ab75d9e..0000000 --- a/tests/unit/test_num_parse.c +++ /dev/null @@ -1,37 +0,0 @@ -#include <stdlib.h> - -#include "testutils.h" -#include "misc.h" - -static void -assert_num_parse (const gchar *str, long expected) -{ - long n; - gboolean res; - - res = num_parse_int(str, &n); - - assert_gboolean(res, TRUE, "Parsing %s failed", str); - assert_gint64(n, expected, "Parsing %s failed", str); -} - -int -main(int argc, char *argv[]) -{ - assert_num_parse("1234", 1234); - assert_num_parse("+1234", 1234); - assert_num_parse("-1234", -1234); - - assert_num_parse("1K", 1000); - assert_num_parse("1k", 1000); - assert_num_parse("1kB", 1000); - assert_num_parse("1Kb", 1000); - - assert_num_parse("1m", 1000 * 1000); - assert_num_parse("1M", 1000 * 1000); - - assert_num_parse("1G", 1000 * 1000 * 1000); - assert_num_parse("1g", 1000 * 1000 * 1000); - - return 0; -}
Any comments? On Wed, Jul 31, 2013 at 7:52 PM, Balazs Scheidler <bazsi@balabit.hu> wrote:
Hi,
I've received your patch via the merge-queue/3.5 branch:
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).
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/misc.c now, and enhanced to recognise the unit suffixes too.
This also closes #28.
Signed-off-by: Gergely Nagy <algernon@balabit.hu>
I really like the change as it definitely seems useful, however I'd have some things I'd like changed, and I've coded this up in a branch named numeric-suffixes.
Could you please fold the patches and resubmit them?
Thanks.
This is the patch I had in mind
commit 3fb2cfbbefe5b3d8f2952e807aa40a7b207a7a68 Author: Balazs Scheidler <bazsi@balabit.hu> Date: Tue Jul 30 16:37:31 2013 +0200
parse-number: follow up patch to number-units functionality
This patch improves the patch series in a number of ways: - moves the number parsing to its separate module - removes the conflicting rule from the lexer, which would never match - accepts 'i' modifier in suffixes to indicate base2 units - string related functions don't get the suffixes functionality - extends the test suite, and adds test functions to indicate the intent of various asserts - test-num-parse is moved under lib/tests
This patch should be folded back into the base set before integration.
diff --git a/lib/Makefile.am b/lib/Makefile.am index d16f755..44ee107 100644 --- a/lib/Makefile.am +++ b/lib/Makefile.am @@ -62,6 +62,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 \ @@ -135,6 +136,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/pragma-parser.c \ diff --git a/lib/cfg-lex.l b/lib/cfg-lex.l index ed61c6f..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,14 +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}+(M|m|G|g|k|K)?(b|B)? { - if (!num_parse_int(yytext, &yylval->num)) +(-|\+)?{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; } -(-|\+)?{digit}+ { yylval->num = strtoll(yytext, NULL, 10); return LL_NUMBER; } ({word}+(\.)?)*{word}+ { return cfg_lexer_lookup_keyword(yyextra, yylval, yylloc, yytext); } \( | \) | diff --git a/lib/misc.c b/lib/misc.c index 2d95c78..7a5fad6 100644 --- a/lib/misc.c +++ b/lib/misc.c @@ -635,48 +635,3 @@ utf8_escape_string(const gchar *str, gssize len)
return res; } - -gboolean -num_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) - return FALSE; - - switch (*endptr) - { - case 'G': - case 'g': - val *= 1000; - case 'm': - case 'M': - val *= 1000; - case 'K': - case 'k': - val *= 1000; - endptr++; - break; - case '\0': - break; - default: - return FALSE; - } - - if (*endptr == 'b' || *endptr == 'B') - endptr++; - - if (*endptr != '\0') - return FALSE; - - *d = val; - return TRUE; -} 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 7117983..4d88daa 100644 --- a/lib/tests/Makefile.am +++ b/lib/tests/Makefile.am @@ -1,4 +1,8 @@ -lib_tests_TESTS = lib/tests/test_log_message lib/tests/test_cfg_lexer_subst +lib_tests_TESTS = \ + lib/tests/test_log_message \ + lib/tests/test_cfg_lexer_subst \ + lib/tests/test_parse_number + check_PROGRAMS += ${lib_tests_TESTS}
lib_tests_test_log_message_CFLAGS = \ @@ -10,3 +14,8 @@ lib_tests_test_cfg_lexer_subst_CFLAGS = \ $(TEST_CFLAGS) lib_tests_test_cfg_lexer_subst_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 c519d86..26fa8bd 100644 --- a/modules/basicfuncs/basic-funcs.c +++ b/modules/basicfuncs/basic-funcs.c @@ -26,7 +26,7 @@ #include "filter/filter-expr.h" #include "filter/filter-expr-parser.h" #include "cfg.h" -#include "misc.h" +#include "parse-number.h" #include "str-format.h" #include "plugin-types.h"
diff --git a/modules/basicfuncs/numeric-funcs.c b/modules/basicfuncs/numeric-funcs.c index 39e4d83..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 (!num_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 (!num_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 b0bcdd4..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 (!num_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 (!num_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; } diff --git a/tests/unit/Makefile.am b/tests/unit/Makefile.am index d5c8ac6..8841834 100644 --- a/tests/unit/Makefile.am +++ b/tests/unit/Makefile.am @@ -15,8 +15,7 @@ tests_unit_TESTS = \ tests/unit/test_logwriter \ tests/unit/test_zone \ tests/unit/test_persist_state \ - tests/unit/test_value_pairs \ - tests/unit/test_num_parse + tests/unit/test_value_pairs
check_PROGRAMS += \ ${tests_unit_TESTS} @@ -81,10 +80,6 @@ tests_unit_test_persist_state_LDADD = \ tests_unit_test_value_pairs_LDADD = \ $(TEST_LDADD) $(unit_test_extra_modules)
-tests_unit_test_num_parse_CFLAGS = ${TEST_CFLAGS} -tests_unit_test_num_parse_LDADD = \ - $(TEST_LDADD) - CLEANFILES += \ test_values.persist \ test_values.persist- diff --git a/tests/unit/test_num_parse.c b/tests/unit/test_num_parse.c deleted file mode 100644 index ab75d9e..0000000 --- a/tests/unit/test_num_parse.c +++ /dev/null @@ -1,37 +0,0 @@ -#include <stdlib.h> - -#include "testutils.h" -#include "misc.h" - -static void -assert_num_parse (const gchar *str, long expected) -{ - long n; - gboolean res; - - res = num_parse_int(str, &n); - - assert_gboolean(res, TRUE, "Parsing %s failed", str); - assert_gint64(n, expected, "Parsing %s failed", str); -} - -int -main(int argc, char *argv[]) -{ - assert_num_parse("1234", 1234); - assert_num_parse("+1234", 1234); - assert_num_parse("-1234", -1234); - - assert_num_parse("1K", 1000); - assert_num_parse("1k", 1000); - assert_num_parse("1kB", 1000); - assert_num_parse("1Kb", 1000); - - assert_num_parse("1m", 1000 * 1000); - assert_num_parse("1M", 1000 * 1000); - - assert_num_parse("1G", 1000 * 1000 * 1000); - assert_num_parse("1g", 1000 * 1000 * 1000); - - return 0; -}
______________________________________________________________________________ Member info: https://lists.balabit.hu/mailman/listinfo/syslog-ng Documentation: http://www.balabit.com/support/documentation/?product=syslog-ng FAQ: http://www.balabit.com/wiki/syslog-ng-faq
-- Bazsi
Balazs Scheidler <bazsi77@gmail.com> writes:
Any comments?
I will rework the patches in a few days and resubmit as requested, once the type-hinting patches land on master. -- |8]
participants (3)
-
Balazs Scheidler
-
Balazs Scheidler
-
Gergely Nagy