Forgot to mention changes in v2: * Use single exit point * Use intermediate variables in size * Change atoll to strtoll and add error check * Changed return type to int * Change result size type to uint64_t (matches LTTng type) * Modify popt subbuf-size argument type to string
Thanks, Simon On 3 April 2013 21:30, Simon Marchi <[email protected]> wrote: > --subbuf-size accepts sizes such as: > > - 123 -> 123 > - 123k -> 123 * 1024 > - 123M -> 123 * 1024 * 1024 > - 123G -> 123 * 1024 * 1024 * 1024 > > It uses the new parse_human_size function, which could probably be used > at other places, such as tracefile size. > > I wanted to add some unit tests for parse_human_size, but the current > test system does not make it easy. They will come later. > > Signed-off-by: Simon Marchi <[email protected]> > --- > src/bin/lttng/commands/enable_channels.c | 21 ++++- > src/bin/lttng/utils.c | 129 > +++++++++++++++++++++++++++++- > src/bin/lttng/utils.h | 6 ++ > 3 files changed, 152 insertions(+), 4 deletions(-) > > diff --git a/src/bin/lttng/commands/enable_channels.c > b/src/bin/lttng/commands/enable_channels.c > index d026af4..c2e9c89 100644 > --- a/src/bin/lttng/commands/enable_channels.c > +++ b/src/bin/lttng/commands/enable_channels.c > @@ -66,7 +66,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}, > @@ -289,6 +289,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(); > > @@ -309,8 +310,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 (!parse_human_size(opt_arg, > &chan.attr.subbuf_size)) { > + 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/bin/lttng/utils.c b/src/bin/lttng/utils.c > index b7f5170..d17fa9c 100644 > --- a/src/bin/lttng/utils.c > +++ b/src/bin/lttng/utils.c > @@ -16,9 +16,11 @@ > */ > > #define _GNU_SOURCE > -#include <stdlib.h> > +#include <assert.h> > #include <ctype.h> > #include <limits.h> > +#include <regex.h> > +#include <stdlib.h> > > #include <common/error.h> > > @@ -77,3 +79,128 @@ void list_cmd_options(FILE *ofp, struct poptOption > *options) > } > } > } > + > +/** > + * 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. > + */ > +void regex_print_error(int errcode, regex_t *regex) > +{ > + /* Get length of error message and allocate accordingly */ > + size_t length = regerror(errcode, regex, NULL, 0); > + char *buffer = malloc(length); > + > + if (buffer) { > + /* Get and print error message */ > + regerror(errcode, regex, buffer, length); > + ERR("regex error: %s\n", buffer); > + free(buffer); > + } else { > + ERR("regex_print_error: malloc failed"); > + } > +} > + > +/** > + * Parse a string that represents a size in human readable format. It > + * support decimal integers suffixed by 'k', 'M' or 'G'. > + * > + * The suffix multiply the integer by: > + * 'k': 1024 > + * 'M': 1024 * 1024 > + * 'G': 1024 * 1024 * 1024 > + * > + * @param str The string to parse. > + * @param size Pointer to a size_t that will be filled with the > + * resulting size. > + * > + * @return 1 on success, 0 on failure. > + */ > +int parse_human_size(char *str, uint64_t *size) > +{ > + regex_t regex; > + int ret; > + const int nmatch = 2; > + regmatch_t matches[nmatch]; > + > + if (!str) > + return 0; > + > + /* Compile regex */ > + ret = regcomp(®ex, "^[0-9][0-9]*\\([kKMG]\\{0,1\\}\\)$", 0); > + > + /* Check for regex error */ > + if (ret != 0) { > + regex_print_error(ret, ®ex); > + > + ret = 0; > + goto end; > + } > + > + /* Match regex */ > + ret = regexec(®ex, str, nmatch, matches, 0); > + if (ret == 0) { > + /* There is a match ! */ > + long long base_size; > + long multiplier = 1; > + errno = 0; > + base_size = strtoll(str, NULL, 10); > + > + /* Check result of conversion */ > + if (errno == ERANGE) { > + ERR("parse_human_size: the value %s caused strtol to > overflow. " > + "Please use a smaller numerical > value.", str); > + ret = 0; > + goto end; > + } else if (errno == EINVAL) { > + ERR("parse_human_size: strtol failed (%s)", str); > + ret = 0; > + goto end; > + } > + > + /* We should be safe because of the regex */ > + assert(base_size > 0); > + > + /* Check if there is a suffix */ > + regmatch_t suffix_match = matches[1]; > + if (suffix_match.rm_eo - suffix_match.rm_so > 0) { > + switch (*(str + suffix_match.rm_so)) { > + case 'K': > + case 'k': > + multiplier = KIBI; > + break; > + case 'M': > + multiplier = MEBI; > + break; > + case 'G': > + multiplier = GIBI; > + break; > + default: > + ERR("parse_human_size: invalid suffix"); > + ret = 0; > + goto end; > + } > + } > + > + *size = base_size * multiplier; > + > + /* Check for overflow */ > + if (*size / base_size != multiplier) { > + ERR("parse_human_size: oops, overflow detected."); > + ret = 0; > + goto end; > + } > + > + ret = 1; > + goto end; > + } else { > + /* There is no match (or error) */ > + ret = 0; > + goto end; > + } > + > +end: > + regfree(®ex); > + return ret; > +} > diff --git a/src/bin/lttng/utils.h b/src/bin/lttng/utils.h > index 9f7bfcc..cb57b93 100644 > --- a/src/bin/lttng/utils.h > +++ b/src/bin/lttng/utils.h > @@ -19,8 +19,14 @@ > #define _LTTNG_UTILS_H > > #include <popt.h> > +#include <stdint.h> > + > +#define KIBI 1024 > +#define MEBI (1024 * 1024) > +#define GIBI (1024 * 1024 * 1024) > > char *get_session_name(void); > void list_cmd_options(FILE *ofp, struct poptOption *options); > +int parse_human_size(char *str, uint64_t *size); > > #endif /* _LTTNG_UTILS_H */ > -- > 1.7.1 > _______________________________________________ lttng-dev mailing list [email protected] http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
