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/lib/libutil/expand_number.3 b/lib/libutil/expand_number.3
index 3609b1ad3939..1b932400de69 100644
--- a/lib/libutil/expand_number.3
+++ b/lib/libutil/expand_number.3
@@ -1,5 +1,6 @@
.\" Copyright (c) 2007 Eric Anderson <ander...@freebsd.org>
.\" Copyright (c) 2007 Pawel Jakub Dawidek <p...@freebsd.org>
+.\" Copyright (c) 2025 Dag-Erling Smørgrav <d...@freebsd.org>
.\" All rights reserved.
.\"
.\" Redistribution and use in source and binary forms, with or without
@@ -23,43 +24,49 @@
.\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
.\" SUCH DAMAGE.
.\"
-.Dd June 13, 2023
+.Dd July 25, 2025
.Dt EXPAND_NUMBER 3
.Os
.Sh NAME
.Nm expand_number
-.Nd format a number from human readable form
+.Nd parse a number from human readable form
.Sh LIBRARY
.Lb libutil
.Sh SYNOPSIS
.In libutil.h
.Ft int
.Fo expand_number
-.Fa "const char *buf" "uint64_t *num"
+.Fa "const char *buf" "int64_t *num"
.Fc
.Sh DESCRIPTION
The
.Fn expand_number
-function parses the
+function parses the number in the string pointed to by its
.Fa buf
-string and stores a unsigned 64-bit quantity at
-.Fa *num .
+argument and stores the number it represents as a signed 64-bit
+quantity in the location pointed to by its
+.Fa *num
+argument.
.Pp
-The
-.Fn expand_number
-function
-is case-insensitive and
-follows the SI power of two convention.
+The input string must consist of a decimal number, optionally preceded
+by a
+.Sq +
+or
+.Sq -
+sign, and optionally followed, without intervening whitespace, by a
+suffix indicating a power-of-two multiplier to apply.
+Any amount of whitespace at the beginning of the string will be
+ignored.
.Pp
-The suffixes are:
+Recognized suffixes are:
.Bl -column "Suffix" "Description" "1000000000000000000" -offset indent
.It Sy "Suffix" Ta Sy "Description" Ta Sy "Multiplier"
-.It Li K Ta No kilo Ta 1024
-.It Li M Ta No mega Ta 1048576
-.It Li G Ta No giga Ta 1073741824
-.It Li T Ta No tera Ta 1099511627776
-.It Li P Ta No peta Ta 1125899906842624
-.It Li E Ta No exa Ta 1152921504606846976
+.It Li K Ta No kilo Ta 1,024
+.It Li M Ta No mega Ta 1,048,576
+.It Li G Ta No giga Ta 1,073,741,824
+.It Li T Ta No tera Ta 1,099,511,627,776
+.It Li P Ta No peta Ta 1,125,899,906,842,624
+.It Li E Ta No exa Ta 1,152,921,504,606,846,976
.El
.Pp
For historical reasons, the
@@ -68,7 +75,11 @@ function accepts and ignores a single
.Dq B
suffix at the end of the
.Fa buf
-string.
+string (i.e.
+.Dq 5b
+is interpreted as 5, and
+.Dq 5kb
+is interpreted as 5,120).
However, the usage of this suffix is discouraged.
.Sh RETURN VALUES
.Rv -std
@@ -78,11 +89,12 @@ The
function will fail if:
.Bl -tag -width Er
.It Bq Er EINVAL
-The given string contains no digits.
+The given string does not contain a valid number.
.It Bq Er EINVAL
-An unrecognized suffix was given.
+An unrecognized suffix was encountered.
.It Bq Er ERANGE
-Result doesn't fit into 64 bits.
+The given string represents a number which does not fit into a
+.Vt int64_t .
.El
.Sh SEE ALSO
.Xr humanize_number 3
diff --git a/lib/libutil/expand_number.c b/lib/libutil/expand_number.c
index fc2ea8e8b17c..f4c19d7867a3 100644
--- a/lib/libutil/expand_number.c
+++ b/lib/libutil/expand_number.c
@@ -3,6 +3,7 @@
*
* Copyright (c) 2007 Eric Anderson <ander...@freebsd.org>
* Copyright (c) 2007 Pawel Jakub Dawidek <p...@freebsd.org>
+ * Copyright (c) 2025 Dag-Erling Smørgrav <d...@freebsd.org>
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
@@ -28,79 +29,120 @@
*/
#include <sys/types.h>
+
#include <ctype.h>
#include <errno.h>
#include <inttypes.h>
#include <libutil.h>
+#include <stdbool.h>
#include <stdint.h>
int
-expand_number(const char *buf, uint64_t *num)
+expand_number(const char *buf, int64_t *num)
{
char *endptr;
- uintmax_t umaxval;
- uint64_t number;
- unsigned shift;
+ uintmax_t number;
+ unsigned int shift;
+ bool neg;
int serrno;
+ /*
+ * Skip whitespace and optional sign.
+ */
+ while (isspace((unsigned char)*buf))
+ buf++;
+ if (*buf == '-') {
+ neg = true;
+ buf++;
+ } else {
+ neg = false;
+ if (*buf == '+')
+ buf++;
+ }
+
+ /*
+ * The next character should be the first digit of the number. If
+ * we don't enforce this ourselves, strtoumax() will allow further
+ * whitespace and a (second?) sign.
+ */
+ if (!isdigit((unsigned char)*buf)) {
+ errno = EINVAL;
+ return (-1);
+ }
+
serrno = errno;
errno = 0;
- umaxval = strtoumax(buf, &endptr, 0);
- if (umaxval > UINT64_MAX)
- errno = ERANGE;
+ number = strtoumax(buf, &endptr, 0);
if (errno != 0)
return (-1);
errno = serrno;
- number = umaxval;
switch (tolower((unsigned char)*endptr)) {
case 'e':
shift = 60;
+ endptr++;
break;
case 'p':
shift = 50;
+ endptr++;
break;
case 't':
shift = 40;
+ endptr++;
break;
case 'g':
shift = 30;
+ endptr++;
break;
case 'm':
shift = 20;
+ endptr++;
break;
case 'k':
shift = 10;
+ endptr++;
break;
- case 'b':
- shift = 0;
- break;
- case '\0': /* No unit. */
- *num = number;
- return (0);
default:
- /* Unrecognized unit. */
- errno = EINVAL;
- return (-1);
+ shift = 0;
}
/*
* Treat 'b' as an ignored suffix for all unit except 'b',
* otherwise there should be no remaining character(s).
*/
- endptr++;
- if (shift != 0 && tolower((unsigned char)*endptr) == 'b')
+ if (tolower((unsigned char)*endptr) == 'b')
endptr++;
if (*endptr != '\0') {
errno = EINVAL;
return (-1);
}
+ /*
+ * Apply the shift and check for overflow.
+ */
if ((number << shift) >> shift != number) {
/* Overflow */
errno = ERANGE;
return (-1);
}
- *num = number << shift;
+ number <<= shift;
+
+ /*
+ * Apply the sign and check for overflow.
+ */
+ if (neg) {
+ if (number > 0x8000000000000000LLU /* -INT64_MIN */) {
+ errno = ERANGE;
+ return (-1);
+ }
+ *num = -number;
+ } else {
+ if (number > INT64_MAX) {
+ errno = ERANGE;
+ return (-1);
+ }
+ *num = number;
+ }
+
return (0);
}
diff --git a/lib/libutil/libutil.h b/lib/libutil/libutil.h
index 7d8bfdf67fac..6d36a0c291c6 100644
--- a/lib/libutil/libutil.h
+++ b/lib/libutil/libutil.h
@@ -89,7 +89,7 @@ __BEGIN_DECLS
char *auth_getval(const char *_name);
void clean_environment(const char * const *_white,
const char * const *_more_white);
-int expand_number(const char *_buf, uint64_t *_num);
+int expand_number(const char *_buf, int64_t *_num);
int extattr_namespace_to_string(int _attrnamespace, char **_string);
int extattr_string_to_namespace(const char *_string, int *_attrnamespace);
int flopen(const char *_path, int _flags, ...);
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);
}
diff --git a/usr.sbin/ctld/parse.y b/usr.sbin/ctld/parse.y
index 56455d33bd27..c0146262e895 100644
--- a/usr.sbin/ctld/parse.y
+++ b/usr.sbin/ctld/parse.y
@@ -105,7 +105,7 @@ statement:
debug: DEBUG STR
{
- uint64_t tmp;
+ int64_t tmp;
if (expand_number($2, &tmp) != 0) {
yyerror("invalid numeric value");
@@ -120,7 +120,7 @@ debug: DEBUG STR
timeout: TIMEOUT STR
{
- uint64_t tmp;
+ int64_t tmp;
if (expand_number($2, &tmp) != 0) {
yyerror("invalid numeric value");
@@ -135,7 +135,7 @@ timeout: TIMEOUT STR
maxproc: MAXPROC STR
{
- uint64_t tmp;
+ int64_t tmp;
if (expand_number($2, &tmp) != 0) {
yyerror("invalid numeric value");
@@ -172,7 +172,7 @@ isns_server: ISNS_SERVER STR
isns_period: ISNS_PERIOD STR
{
- uint64_t tmp;
+ int64_t tmp;
if (expand_number($2, &tmp) != 0) {
yyerror("invalid numeric value");
@@ -187,7 +187,7 @@ isns_period: ISNS_PERIOD STR
isns_timeout: ISNS_TIMEOUT STR
{
- uint64_t tmp;
+ int64_t tmp;
if (expand_number($2, &tmp) != 0) {
yyerror("invalid numeric value");
@@ -432,7 +432,7 @@ portal_group_redirect: REDIRECT STR
portal_group_tag: TAG STR
{
- uint64_t tmp;
+ int64_t tmp;
if (expand_number($2, &tmp) != 0) {
yyerror("invalid numeric value");
@@ -448,7 +448,7 @@ portal_group_tag: TAG STR
portal_group_dscp
: DSCP STR
{
- uint64_t tmp;
+ int64_t tmp;
if (strcmp($2, "0x") == 0) {
tmp = strtol($2 + 2, NULL, 16);
@@ -488,7 +488,7 @@ portal_group_dscp
portal_group_pcp: PCP STR
{
- uint64_t tmp;
+ int64_t tmp;
if (expand_number($2, &tmp) != 0) {
yyerror("invalid numeric value");
@@ -704,7 +704,7 @@ target_lun: LUN lun_number
lun_number: STR
{
- uint64_t tmp;
+ int64_t tmp;
if (expand_number($1, &tmp) != 0) {
yyerror("invalid numeric value");
@@ -720,7 +720,7 @@ lun_number: STR
target_lun_ref: LUN STR STR
{
- uint64_t tmp;
+ int64_t tmp;
bool ok;
if (expand_number($2, &tmp) != 0) {
@@ -778,7 +778,7 @@ lun_backend: BACKEND STR
lun_blocksize: BLOCKSIZE STR
{
- uint64_t tmp;
+ int64_t tmp;
if (expand_number($2, &tmp) != 0) {
yyerror("invalid numeric value");
@@ -816,7 +816,7 @@ lun_device_type: DEVICE_TYPE STR
lun_ctl_lun: CTL_LUN STR
{
- uint64_t tmp;
+ int64_t tmp;
if (expand_number($2, &tmp) != 0) {
yyerror("invalid numeric value");
@@ -866,7 +866,7 @@ lun_serial: SERIAL STR
lun_size: SIZE STR
{
- uint64_t tmp;
+ int64_t tmp;
if (expand_number($2, &tmp) != 0) {
yyerror("invalid numeric value");
