Re: [PATCH v2] dm cache: parse invalidate_cblocks with kstrtouint()

On Wed, Jul 8, 2026 at 8:54 AM Samuel Moelius
<[email protected]> wrote:
>
> -static int parse_cblock_range(struct cache *cache, const char *str,
> +static int parse_cblock_range(struct cache *cache, char *str,
>                               struct cblock_range *result)
>  {
> -       char dummy;
> -       uint64_t b, e;
> +       char *begin = strsep(&str, "-");
> +       unsigned int b, e;
>         int r;
>
> -       /*
> -        * Try and parse form (ii) first.
> -        */
> -       r = sscanf(str, "%llu-%llu%c", &b, &e, &dummy);
> +       if (str) {
> +               r = kstrtouint(begin, 10, &b);
> +               if (r)
> +                       goto bad;
> +
> +               r = kstrtouint(str, 10, &e);
> +               if (r)
> +                       goto bad;
>
> -       if (r == 2) {
>                 result->begin = to_cblock(b);
>                 result->end = to_cblock(e);
>                 return 0;
>         }
>
> -       /*
> -        * That didn't work, try form (i).
> -        */
> -       r = sscanf(str, "%llu%c", &b, &dummy);
> -
> -       if (r == 1) {
> +       r = kstrtouint(begin, 10, &b);

This duplicates the kstrtouint(begin) call below. I'd prefer parse
begin once, then branch on whether str is non-NULL.

> +       if (!r) {
>                 result->begin = to_cblock(b);
> -               result->end = to_cblock(from_cblock(result->begin) + 1u);
> +               result->end = cblock_succ(result->begin);
>                 return 0;
>         }

This should follow the same early-return-on-error pattern above: goto
bad on failure, and keep the happy path unindented.

> -       DMERR("%s: invalid cblock range '%s'", cache_device_name(cache), str);
> +bad:
> +       DMERR("%s: invalid cblock range '%s%s%s'", cache_device_name(cache),
> +             begin, str ? "-" : "", str ?: "");

Since strsep() is destructive, printing the rejected token is simpler
and more useful for debugging. How about:

blocknr = strsep(&str)
r = strtouint(blocknr)
...
if (str) {
  blocknr = str
  r = strtouint(blocknr)
  ...
} else {
  ... // single-value case
}

DMERR("%s", blocknr)