From 70d0b120691e90d81de7b38af8b845e261b5b40c Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Fri, 12 Apr 2013 17:15:12 -0400 Subject: [PATCH 1/1] lttng cli: Accept human readable sizes for --subbuf-size --subbuf-size accepts sizes such as: - 123 -> 123 - 123k -> 123 * 1024 - 123M -> 123 * 1024^2 - 123G -> 123 * 1024^3 It uses the new parse_size_suffix function, which could probably be used at other places, such as tracefile size. Unit tests are included. Reviewed-by: Christian Babeux Signed-off-by: Simon Marchi Signed-off-by: David Goulet --- .gitignore | 1 + doc/man/lttng.1 | 2 +- src/bin/lttng/commands/enable_channels.c | 24 ++++- src/common/utils.c | 124 ++++++++++++++++++++++ src/common/utils.h | 9 ++ tests/unit/Makefile.am | 15 ++- tests/unit/test_utils_parse_size_suffix.c | 87 +++++++++++++++ 7 files changed, 254 insertions(+), 8 deletions(-) create mode 100644 tests/unit/test_utils_parse_size_suffix.c diff --git a/.gitignore b/.gitignore index 8e2e63d1c..120d93658 100644 --- a/.gitignore +++ b/.gitignore @@ -53,6 +53,7 @@ tests/unit/test_kernel_data tests/unit/test_session tests/unit/test_uri tests/unit/test_ust_data +tests/unit/test_utils_parse_size_suffix kernel_all_events_basic kernel_event_basic ust_global_event_wildcard diff --git a/doc/man/lttng.1 b/doc/man/lttng.1 index 41885a854..65fa3d17f 100644 --- a/doc/man/lttng.1 +++ b/doc/man/lttng.1 @@ -327,7 +327,7 @@ same type. \-\-overwrite Flight recorder mode : overwrites events when subbuffers are full \-\-subbuf-size SIZE - Subbuffer size in bytes (default: 4096, kernel default: 262144) + Subbuffer size in bytes {+k,+M,+G} (default: 4096, kernel default: 262144) Needs to be a power of 2 for both tracers \-\-num-subbuf NUM Number of subbuffers (default: 4) diff --git a/src/bin/lttng/commands/enable_channels.c b/src/bin/lttng/commands/enable_channels.c index fbf02a058..039750bcb 100644 --- a/src/bin/lttng/commands/enable_channels.c +++ b/src/bin/lttng/commands/enable_channels.c @@ -28,6 +28,7 @@ #include "../command.h" #include +#include static char *opt_channels; static int opt_kernel; @@ -66,7 +67,7 @@ static struct poptOption long_options[] = { {"userspace", 'u', POPT_ARG_NONE, 0, OPT_USERSPACE, 0, 0}, {"discard", 0, POPT_ARG_NONE, 0, OPT_DISCARD, 0, 0}, {"overwrite", 0, POPT_ARG_NONE, 0, OPT_OVERWRITE, 0, 0}, - {"subbuf-size", 0, POPT_ARG_DOUBLE, 0, OPT_SUBBUF_SIZE, 0, 0}, + {"subbuf-size", 0, POPT_ARG_STRING, 0, OPT_SUBBUF_SIZE, 0, 0}, {"num-subbuf", 0, POPT_ARG_INT, 0, OPT_NUM_SUBBUF, 0, 0}, {"switch-timer", 0, POPT_ARG_INT, 0, OPT_SWITCH_TIMER, 0, 0}, {"read-timer", 0, POPT_ARG_INT, 0, OPT_READ_TIMER, 0, 0}, @@ -99,7 +100,7 @@ static void usage(FILE *ofp) DEFAULT_CHANNEL_OVERWRITE ? "" : " (default)"); fprintf(ofp, " --overwrite Flight recorder mode%s\n", DEFAULT_CHANNEL_OVERWRITE ? " (default)" : ""); - fprintf(ofp, " --subbuf-size SIZE Subbuffer size in bytes\n"); + fprintf(ofp, " --subbuf-size SIZE Subbuffer size in bytes {+k,+M,+G}\n"); fprintf(ofp, " (default: %zu, kernel default: %zu)\n", default_get_channel_subbuf_size(), default_get_kernel_channel_subbuf_size()); @@ -299,6 +300,7 @@ int cmd_enable_channels(int argc, const char **argv) int opt, ret = CMD_SUCCESS; static poptContext pc; char *session_name = NULL; + char *opt_arg = NULL; init_channel_config(); @@ -319,8 +321,22 @@ int cmd_enable_channels(int argc, const char **argv) DBG("Channel set to overwrite"); break; case OPT_SUBBUF_SIZE: - /* TODO Replace atol with strtol and check for errors */ - chan.attr.subbuf_size = atol(poptGetOptArg(pc)); + /* Parse the size */ + opt_arg = poptGetOptArg(pc); + if (utils_parse_size_suffix(opt_arg, &chan.attr.subbuf_size) < 0) { + ERR("Wrong value the --subbuf-size parameter: %s", opt_arg); + ret = CMD_ERROR; + goto end; + } + + /* Check if power of 2 */ + if ((chan.attr.subbuf_size - 1) & chan.attr.subbuf_size) { + ERR("The subbuf size is not a power of 2: %" PRIu64 " (%s)", + chan.attr.subbuf_size, opt_arg); + ret = CMD_ERROR; + goto end; + } + DBG("Channel subbuf size set to %" PRIu64, chan.attr.subbuf_size); break; case OPT_NUM_SUBBUF: diff --git a/src/common/utils.c b/src/common/utils.c index 1c2b95d3e..f3718f00f 100644 --- a/src/common/utils.c +++ b/src/common/utils.c @@ -26,6 +26,7 @@ #include #include #include +#include #include #include @@ -394,3 +395,126 @@ int utils_rotate_stream_file(char *path_name, char *file_name, uint64_t size, 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'. + * + * The suffix multiply the integer by: + * 'k': 1024 + * 'M': 1024^2 + * 'G': 1024^3 + * + * @param str The string to parse. + * @param size Pointer to a size_t that will be filled with the + * resulting size. + * + * @return 0 on success, -1 on failure. + */ +int utils_parse_size_suffix(char *str, uint64_t *size) +{ + regex_t regex; + int ret; + const int nmatch = 3; + regmatch_t suffix_match, matches[nmatch]; + unsigned long long base_size; + long shift = 0; + + 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); + ret = -1; + goto end; + } + + /* Match regex */ + ret = regexec(®ex, str, nmatch, matches, 0); + if (ret != 0) { + ret = -1; + goto free; + } + + /* There is a match ! */ + errno = 0; + base_size = strtoull(str, NULL, 0); + if (errno != 0) { + PERROR("strtoull"); + ret = -1; + goto free; + } + + /* 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; + } + } + + *size = base_size << shift; + + /* Check for overflow */ + if ((*size >> shift) != base_size) { + ERR("parse_size_suffix: oops, overflow detected."); + ret = -1; + goto free; + } + + ret = 0; + +free: + regfree(®ex); +end: + return ret; +} diff --git a/src/common/utils.h b/src/common/utils.h index 2d39cefb9..d8a5321c7 100644 --- a/src/common/utils.h +++ b/src/common/utils.h @@ -18,6 +18,14 @@ #ifndef _COMMON_UTILS_H #define _COMMON_UTILS_H +#include +#include +#include + +#define KIBI_LOG2 10 +#define MEBI_LOG2 20 +#define GIBI_LOG2 30 + char *utils_expand_path(const char *path); int utils_create_pipe(int *dst); int utils_create_pipe_cloexec(int *dst); @@ -30,5 +38,6 @@ int utils_create_stream_file(char *path_name, char *file_name, uint64_t size, uint64_t count, int uid, int gid); 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 utils_parse_size_suffix(char *str, uint64_t *size); #endif /* _COMMON_UTILS_H */ diff --git a/tests/unit/Makefile.am b/tests/unit/Makefile.am index 67e7fe454..e8a6429d8 100644 --- a/tests/unit/Makefile.am +++ b/tests/unit/Makefile.am @@ -14,10 +14,11 @@ LIBCOMMON=$(top_builddir)/src/common/libcommon.la LIBSESSIOND_COMM=$(top_builddir)/src/common/sessiond-comm/libsessiond-comm.la LIBHASHTABLE=$(top_builddir)/src/common/hashtable/libhashtable.la +# Define test programs +noinst_PROGRAMS = test_uri test_session test_kernel_data test_utils_parse_size_suffix + if HAVE_LIBLTTNG_UST_CTL -noinst_PROGRAMS = test_uri test_session test_ust_data test_kernel_data -else -noinst_PROGRAMS = test_uri test_session test_kernel_data +noinst_PROGRAMS += test_ust_data endif # URI unit tests @@ -69,3 +70,11 @@ test_kernel_data_SOURCES = test_kernel_data.c test_kernel_data_LDADD = $(LIBTAP) $(LIBCOMMON) $(LIBSESSIOND_COMM) $(LIBHASHTABLE) \ -lrt test_kernel_data_LDADD += $(KERN_DATA_TRACE) + +# parse_size_suffix unit test +UTILS_PARSE_SIZE_SUFFIX=$(top_srcdir)/src/common/utils.o \ + $(top_srcdir)/src/common/runas.o + +test_utils_parse_size_suffix_SOURCES = test_utils_parse_size_suffix.c +test_utils_parse_size_suffix_LDADD = $(LIBTAP) $(LIBHASHTABLE) $(LIBCOMMON) +test_utils_parse_size_suffix_LDADD += $(UTILS_PARSE_SIZE_SUFFIX) diff --git a/tests/unit/test_utils_parse_size_suffix.c b/tests/unit/test_utils_parse_size_suffix.c new file mode 100644 index 000000000..3b9c68caf --- /dev/null +++ b/tests/unit/test_utils_parse_size_suffix.c @@ -0,0 +1,87 @@ +/* + * Copyright (C) - 2013 Simon Marchi + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by as + * published by the Free Software Foundation; only version 2 of the License. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along with + * this program; if not, write to the Free Software Foundation, Inc., 51 + * Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#include +#include +#include + +#include + +#include + +/* For lttngerr.h */ +int lttng_opt_quiet = 1; +int lttng_opt_verbose = 3; + +struct valid_test_input { + char *input; + uint64_t expected_result; +}; + +/* Valid test cases */ +static struct valid_test_input valid_tests_inputs[] = { + { "0", 0 }, + { "1234", 1234 }, + { "0x400", 1024 }, + { "0300", 192 }, + { "16k", 16384 }, + { "128K", 131072 }, + { "0x1234k", 4771840 }, + { "32M", 33554432 }, + { "1024G", 1099511627776 }, +}; +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 const int num_invalid_tests = sizeof(invalid_tests_inputs) / sizeof(invalid_tests_inputs[0]); + +static void test_utils_parse_size_suffix(void) +{ + uint64_t result; + int ret; + int i; + + /* Test valid cases */ + for (i = 0; i < num_valid_tests; i++) { + char name[100]; + sprintf(name, "valid test case: %s", valid_tests_inputs[i].input); + + ret = utils_parse_size_suffix(valid_tests_inputs[i].input, &result); + ok(ret == 0 && result == valid_tests_inputs[i].expected_result, name); + } + + /* Test invalid cases */ + for (i = 0; i < num_invalid_tests; i++) { + char name[100]; + sprintf(name, "invalid test case: %s", invalid_tests_inputs[i]); + + ret = utils_parse_size_suffix(invalid_tests_inputs[i], &result); + ok(ret != 0, name); + } +} + +int main(int argc, char **argv) +{ + plan_tests(num_valid_tests + num_invalid_tests); + + diag("utils_parse_size_suffix tests"); + + test_utils_parse_size_suffix(); + + return exit_status(); +} -- 2.34.1