mbox series

[v2,0/2] lib/kstrtox: introduce kstrtoull_suffix() helper

Message ID cover.1703030510.git.wqu@suse.com (mailing list archive)
Headers show
Series lib/kstrtox: introduce kstrtoull_suffix() helper | expand

Message

Qu Wenruo Dec. 20, 2023, 12:09 a.m. UTC
[CHANGELOG]
v2:
- Use enum bitmap to define suffixes
  This get rid of the upper/lower case problem, and using enum for each
  suffix still provides a somewhat readable result.

- Do proper suffix overflow check
  The previous check for suffix overflow is incorrect at all, would
  still lead to incorrect values.

- Slight refactor for kstrtoull_suffix() code
  

Recently David Sterba <dsterba@suse.cz> exposed some weird behavior of
btrfs sysfs interface, which is utilizing memparse().

One example is using "echo 25e >
/sys/fs/btrfs/<uuid>/devinfo/scrub_speed_max".

The result value is not 0x25e, because there is no "0x" prefix provided.
Nor (25 << 60), as that would overflow.

All these are the caveats of memparse(), which lacks:

- Overflow check
- Reasonable suffix selection
  I know there may be some niche cases for memory layout, but for most
  callers, they just want a kstrtoull() with reasonable suffix
  selection.
  And I don't think exabytes is a reasonable suffix.

So here we introduce a new helper, "kstrtoull_suffix", which has some
the following two abilities added:

- Allow caller to select the suffix they want to enable
  The default list is "KkMmGgTtPp", no unreasonable "Ee" ones.

- Allow suffix parsing with overflow detection.
  The int part detection is already done by _kstrtoull(), and with the
  extra left shift, do the check again before returning the value.

Unfortunately this new helper is still not a drop-in replacement for
memparse():

- No @retptr support
  It's possible to add, but I'm not sure how many call sites need that
  for extra separators, and even if there are, they may want to go
  strsep() and strtok().

- Need to check the failure properly
  Which memparse() callers never seems to check anyway.

Thus the existing memparse() callers need to opt-in.
For now, btrfs usage of memparse() can be easily converted to
kstrtoull_suffix(), in the 2nd patch.

Qu Wenruo (2):
  lib/strtox: introduce kstrtoull_suffix() helper
  btrfs: sysfs: use kstrtoull_suffix() to replace memparse()

 fs/btrfs/sysfs.c        |  20 +++-----
 include/linux/kstrtox.h |  20 ++++++++
 lib/kstrtox.c           | 108 ++++++++++++++++++++++++++++++++++++++--
 3 files changed, 131 insertions(+), 17 deletions(-)