On 8/2/25 10:06, Dag-Erling Smørgrav wrote:
The branch main has been updated by des:
URL:
https://cgit.FreeBSD.org/src/commit/?id=2e0caa7c7e14d7bdc89ec43be9bc848abe1ca264
commit 2e0caa7c7e14d7bdc89ec43be9bc848abe1ca264
Author: Dag-Erling Smørgrav <d...@freebsd.org>
AuthorDate: 2025-08-02 14:05:36 +0000
Commit: Dag-Erling Smørgrav <d...@freebsd.org>
CommitDate: 2025-08-02 14:05:36 +0000
libutil: Really fix expand_number(3)
It is unclear whether this function was originally intended to support
negative numbers. The original implementation used signed integers,
but actually giving it a negative number as input would have invoked
undefined behavior, and the comments (since removed) only mentioned
positive numbers. Fifteen years ago, I “fixed” this by changing the
type from signed to unsigned. However, it would still have accepted
an input with a leading minus sign (though it would have returned its
absolute value). Fifteen years on, change the type back to signed and
fix the logic so it correctly handles both positive and negative
numbers without invoking undefined behavior. This makes it a better
match for humanize_number(3), which it is supposed to complement.
Fixes: bbb2703b4f46
Reviewed by: jhb
Differential Revision: https://reviews.freebsd.org/D51542
---
lib/libutil/expand_number.3 | 56 +++++++++++++++++++------------
lib/libutil/expand_number.c | 82 ++++++++++++++++++++++++++++++++++-----------
lib/libutil/libutil.h | 2 +-
usr.sbin/ctld/conf.cc | 12 ++++---
usr.sbin/ctld/parse.y | 26 +++++++-------
5 files changed, 118 insertions(+), 60 deletions(-)
diff --git a/usr.sbin/ctld/conf.cc b/usr.sbin/ctld/conf.cc
index e86b44ee5004..f3285ebf9d56 100644
--- a/usr.sbin/ctld/conf.cc
+++ b/usr.sbin/ctld/conf.cc
@@ -409,7 +409,8 @@ lun_set_blocksize(size_t value)
bool
lun_set_device_type(const char *value)
{
- uint64_t device_type;
+ const char *errstr;
+ int device_type;
if (strcasecmp(value, "disk") == 0 ||
strcasecmp(value, "direct") == 0)
@@ -421,9 +422,12 @@ lun_set_device_type(const char *value)
strcasecmp(value, "dvd") == 0 ||
strcasecmp(value, "dvdrom") == 0)
device_type = T_CDROM;
- else if (expand_number(value, &device_type) != 0 || device_type > 15) {
- log_warnx("invalid device-type \"%s\" for lun \"%s\"", value,
- lun->l_name);
+ else {
+ device_type = strtonum(value, 0, 15, &errstr);
+ if (errstr != NULL) {
+ log_warnx("invalid device-type \"%s\" for lun \"%s\"",
value,
+ lun->l_name);
+ }
return (false);
This breaks any integer device types. The return (false) should only be used in
the error case. I will fix as part of my other ctld changes.
--
John Baldwin
