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 <christian.babeux@efficios.com>
Signed-off-by: Simon Marchi <simon.marchi@polymtl.ca>
Signed-off-by: David Goulet <dgoulet@efficios.com>

diff --git a/.gitignore b/.gitignore
index 8e2e63d1cd1a99f70578f16842b85a3774b191f0..120d93658e36e2db8781d09febde905be8a97d99 100644 (file)
--- 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_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
 kernel_all_events_basic
 kernel_event_basic
 ust_global_event_wildcard

diff --git a/doc/man/lttng.1 b/doc/man/lttng.1
index 41885a8547b3028fdf1fa667fa3c37e7e29052a9..65fa3d17f465e2a2fcd1dbfb4ef0da8fd1d1c477 100644 (file)
--- 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
 \-\-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)
         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 fbf02a0580c520fc37f9baf62cdd3ec2a962387c..039750bcb99dc8c7e95d902e5d26656eb3b8c3ea 100644 (file)
--- a/src/bin/lttng/commands/enable_channels.c
+++ b/src/bin/lttng/commands/enable_channels.c
@@ -28,6 +28,7 @@
 #include "../command.h"
 
 #include <src/common/sessiond-comm/sessiond-comm.h>
 #include "../command.h"
 
 #include <src/common/sessiond-comm/sessiond-comm.h>

+#include <src/common/utils.h>

 
 static char *opt_channels;
 static int opt_kernel;
 
 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},
        {"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},
        {"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)" : "");
                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());
        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;
        int opt, ret = CMD_SUCCESS;
        static poptContext pc;
        char *session_name = NULL;

+       char *opt_arg = NULL;

 
        init_channel_config();
 
 
        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:
                        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:
                        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 1c2b95d3e7d5d3e427bc4c20f59ba4483062dea9..f3718f00fa8ddabb7c921ac55287626dd83f45b8 100644 (file)
--- a/src/common/utils.c
+++ b/src/common/utils.c
@@ -26,6 +26,7 @@
 #include <sys/stat.h>
 #include <unistd.h>
 #include <inttypes.h>
 #include <sys/stat.h>
 #include <unistd.h>
 #include <inttypes.h>

+#include <regex.h>

 
 #include <common/common.h>
 #include <common/runas.h>
 
 #include <common/common.h>
 #include <common/runas.h>


@@ -394,3 +395,126 @@ int utils_rotate_stream_file(char *path_name, char *file_name, uint64_t size,
 error:
        return ret;
 }
 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(&regex, "^\\(0x\\)\\{0,1\\}[0-9][0-9]*\\([kKMG]\\{0,1\\}\\)$", 0);
+       if (ret != 0) {
+               regex_print_error(ret, &regex);
+               ret = -1;
+               goto end;
+       }
+
+       /* Match regex */
+       ret = regexec(&regex, 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(&regex);
+end:
+       return ret;
+}

diff --git a/src/common/utils.h b/src/common/utils.h
index 2d39cefb96b7bfa27c12f2ee1865d294ca0bca9c..d8a5321c7c5799e7b706390cacd93b8cd3e4be1f 100644 (file)
--- a/src/common/utils.h
+++ b/src/common/utils.h
@@ -18,6 +18,14 @@
 #ifndef _COMMON_UTILS_H
 #define _COMMON_UTILS_H
 
 #ifndef _COMMON_UTILS_H
 #define _COMMON_UTILS_H
 

+#include <sys/types.h>
+#include <unistd.h>
+#include <stdint.h>
+
+#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);
 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);
                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 */
 
 #endif /* _COMMON_UTILS_H */

diff --git a/tests/unit/Makefile.am b/tests/unit/Makefile.am
index 67e7fe4545b72aa59904240bcc84a182fea61ccc..e8a6429d8f8446887df9ca62642e600a6593b396 100644 (file)
--- 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
 
 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
 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
 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)
 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 (file)
index 0000000..3b9c68c
--- /dev/null
+++ b/tests/unit/test_utils_parse_size_suffix.c
@@ -0,0 +1,87 @@
+/*
+ * Copyright (C) - 2013 Simon Marchi <simon.marchi@polymtl.ca>
+ *
+ * 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 <assert.h>
+#include <string.h>
+#include <stdio.h>
+
+#include <tap/tap.h>
+
+#include <src/common/utils.h>
+
+/* 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();
+}