[0/2] btrfs-progs: parser related cleanups

Message

Qu Wenruo Jan. 17, 2024, 4:10 a.m. UTC

Btrfs-progs has two types of parsers:

- parse_*()
  Those would return 0 for a good parse, and save the value into a
  pointer.
  Callers are responsible to handle the error.

- arg_strto*()
  Those would directly return the parsed value, and call exit(1)
  directly for errors.

However this split is not perfect:

- A lot of code can be shared between them
  In fact, mostly arg_strto*() can be implement using parse_*().
  The only difference is in how detailed the error string would be.

- parse_size_from_string() doesn't follow the scheme
  It follows arg_strto*() behavior but has the parse_*() name.

This patch would:

- Use parse_u64() to implement arg_strtou64()
  The first patch.

- Use parse_u64_with_suffix() to implement arg_strtou64_with_suffix()
  The new helper parse_u64_with_suffix() would replace the old
  parse_size_from_string() and do the proper error handling.

Qu Wenruo (2):
  btrfs-progs: use parse_u64() to implement arg_strtou64()
  btrfs-progs: implement arg_strtou64_with_suffix() with a new helper

 cmds/filesystem.c     | 15 ++++----
 cmds/qgroup.c         |  3 +-
 cmds/reflink.c        |  7 ++--
 cmds/scrub.c          |  2 +-
 common/parse-utils.c  | 85 ++++++++++++++++++++++++++++---------------
 common/parse-utils.h  |  2 +-
 common/string-utils.c | 45 +++++++++++++++--------
 common/string-utils.h |  3 ++
 convert/main.c        |  3 +-
 kernel-shared/zoned.c |  4 +-
 mkfs/main.c           |  6 +--
 11 files changed, 110 insertions(+), 65 deletions(-)

--
2.43.0

Comments

David Sterba Jan. 17, 2024, 7:34 p.m. UTC | #1

On Wed, Jan 17, 2024 at 02:40:38PM +1030, Qu Wenruo wrote:
> Btrfs-progs has two types of parsers:
> 
> - parse_*()
>   Those would return 0 for a good parse, and save the value into a
>   pointer.
>   Callers are responsible to handle the error.
> 
> - arg_strto*()
>   Those would directly return the parsed value, and call exit(1)
>   directly for errors.
> 
> However this split is not perfect:
> 
> - A lot of code can be shared between them
>   In fact, mostly arg_strto*() can be implement using parse_*().
>   The only difference is in how detailed the error string would be.
> 
> - parse_size_from_string() doesn't follow the scheme
>   It follows arg_strto*() behavior but has the parse_*() name.
> 
> This patch would:
> 
> - Use parse_u64() to implement arg_strtou64()
>   The first patch.
> 
> - Use parse_u64_with_suffix() to implement arg_strtou64_with_suffix()
>   The new helper parse_u64_with_suffix() would replace the old
>   parse_size_from_string() and do the proper error handling.
> 
> Qu Wenruo (2):
>   btrfs-progs: use parse_u64() to implement arg_strtou64()
>   btrfs-progs: implement arg_strtou64_with_suffix() with a new helper

Thanks, that's better than I expected, it's clear what helpers can be
used where just by the name. I did some minor adjustments or added a
comment for the arg_* helper declarations.