public inbox for linux-btrfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Qu Wenruo <wqu@suse.com>
To: linux-btrfs@vger.kernel.org
Subject: [PATCH 0/2] lib/kstrtox: introduce kstrtoull_suffix() helper
Date: Fri, 15 Dec 2023 19:09:22 +1030	[thread overview]
Message-ID: <cover.1702628925.git.wqu@suse.com> (raw)

Recently David Sterba <dsterba@suse.cz> exposed some weird behavior of
btrfs sysfs interface, which is utilize 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.

- 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 |   7 +++
 lib/kstrtox.c           | 113 ++++++++++++++++++++++++++++++++++++++--
 3 files changed, 123 insertions(+), 17 deletions(-)

-- 
2.43.0


             reply	other threads:[~2023-12-15  8:39 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-15  8:39 Qu Wenruo [this message]
2023-12-15  8:39 ` [PATCH 1/2] lib/strtox: introduce kstrtoull_suffix() helper Qu Wenruo
2023-12-18 12:59   ` David Disseldorp
2023-12-18 19:52     ` Qu Wenruo
2023-12-19  3:17       ` David Disseldorp
2023-12-19 16:42     ` David Laight
2023-12-19 21:17       ` Qu Wenruo
2023-12-20  8:31         ` David Laight
2023-12-20  9:32           ` Qu Wenruo
2023-12-15  8:39 ` [PATCH 2/2] btrfs: sysfs: use kstrtoull_suffix() to replace memparse() Qu Wenruo
2023-12-18  7:49   ` David Disseldorp
2023-12-18  8:11     ` Qu Wenruo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cover.1702628925.git.wqu@suse.com \
    --to=wqu@suse.com \
    --cc=linux-btrfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox