[lttng-dev] [PATCH lttng-tools] utils: Rework utils_parse_size_suffix

[Simon Marchi]

Thanks for the comments.

> @@ -693,83 +660,94 @@ static void regex_print_error(int errcode, regex_t *regex)
> [...]
> +       /* strtoull will accept a negative number, but we don't want to. */
> +       if (strchr(str, '-') != NULL) {
> +               DBG("utils_parse_size_suffix: invalid size string, should not contain '-'.");
>                 ret = -1;
> +               goto end;
>         }
>

>>>>   Should be "[...] should not begin with '-' [...]" (although it is true the size string should not *contain*, the test is only for *begins with*)

I meant for it to search in the whole string, and that is what I think
strchr does.

> [...]
> +       if (num_end == str) {
> +               /* strtoull parsed nothing, not good. */
> +               DBG("utils_parse_size_suffix: strtoull had nothing good to parse.\n");
> +               ret = -1;
> +               goto end;
> +       }
>
>>>>   "utils_parse_size_suffix: zero-length size string." would be clearer.  Also note that the preceding DBG strings did not end with '\n'.

This test indeed will catch zero-length strings. However, iif you pass
"hello", or "k", strtoull will just parse 0 characters and return the
value 0. This test is for these cases as well, which are not
zero-length strings. I agree that it's not very clear, but I have
nothing better...