From: Qu Wenruo <wqu@suse.com>
To: linux-btrfs@vger.kernel.org
Subject: [PATCH v2 0/2] lib/kstrtox: introduce kstrtoull_suffix() helper
Date: Wed, 20 Dec 2023 10:39:59 +1030 [thread overview]
Message-ID: <cover.1703030510.git.wqu@suse.com> (raw)
[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(-)
--
2.43.0
next reply other threads:[~2023-12-20 0:10 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-20 0:09 Qu Wenruo [this message]
2023-12-20 0:10 ` [PATCH v2 1/2] lib/strtox: introduce kstrtoull_suffix() helper Qu Wenruo
2023-12-20 5:38 ` David Disseldorp
2023-12-23 9:57 ` Qu Wenruo
2023-12-27 5:38 ` David Disseldorp
2023-12-20 14:17 ` Andy Shevchenko
2023-12-20 0:10 ` [PATCH v2 2/2] btrfs: sysfs: use kstrtoull_suffix() to replace memparse() 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.1703030510.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