Richard W.M. Jones
2023-Sep-03 15:22 UTC
[Libguestfs] [PATCH nbdkit] server: Move size parsing code (nbdkit_parse_size) to common/include
This is broadly simple code motion, intended so that we can reuse the
same code in libnbd.
---
common/include/Makefile.am | 6 ++
common/include/human-size.h | 137 +++++++++++++++++++++++++++++++
common/include/test-human-size.c | 133 ++++++++++++++++++++++++++++++
server/public.c | 78 ++----------------
.gitignore | 1 +
5 files changed, 283 insertions(+), 72 deletions(-)
diff --git a/common/include/Makefile.am b/common/include/Makefile.am
index 3162e92c2..8e4de04f3 100644
--- a/common/include/Makefile.am
+++ b/common/include/Makefile.am
@@ -42,6 +42,7 @@ EXTRA_DIST = \
checked-overflow.h \
compiler-macros.h \
hexdigit.h \
+ human-size.h \
isaligned.h \
ispowerof2.h \
iszero.h \
@@ -63,6 +64,7 @@ TESTS = \
test-ascii-string \
test-byte-swapping \
test-checked-overflow \
+ test-human-size \
test-isaligned \
test-ispowerof2 \
test-iszero \
@@ -93,6 +95,10 @@ test_checked_overflow_SOURCES = test-checked-overflow.c
checked-overflow.h
test_checked_overflow_CPPFLAGS = -I$(srcdir)
test_checked_overflow_CFLAGS = $(WARNINGS_CFLAGS)
+test_human_size_SOURCES = test-human-size.c human-size.h
+test_human_size_CPPFLAGS = -I$(srcdir)
+test_human_size_CFLAGS = $(WARNINGS_CFLAGS)
+
test_isaligned_SOURCES = test-isaligned.c isaligned.h
test_isaligned_CPPFLAGS = -I$(srcdir)
test_isaligned_CFLAGS = $(WARNINGS_CFLAGS)
diff --git a/common/include/human-size.h b/common/include/human-size.h
new file mode 100644
index 000000000..788dbd7ba
--- /dev/null
+++ b/common/include/human-size.h
@@ -0,0 +1,137 @@
+/* nbdkit
+ * Copyright Red Hat
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * * Neither the name of Red Hat nor the names of its contributors may be
+ * used to endorse or promote products derived from this software without
+ * specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS
IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
+ * PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+ * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#ifndef NBDKIT_HUMAN_SIZE_H
+#define NBDKIT_HUMAN_SIZE_H
+
+#include <stdint.h>
+#include <inttypes.h>
+#include <errno.h>
+
+/* Attempt to parse a string with a possible scaling suffix, such as
+ * "2M". Disk sizes cannot usefully exceed off_t (which is signed)
+ * and cannot be negative.
+ *
+ * On error, returns -1 and sets *error and *pstr. You can form a
+ * final error message by appending "<error>: <pstr>".
+ */
+static inline int64_t
+human_size_parse (const char *str,
+ const char **error, const char **pstr)
+{
+ int64_t size;
+ char *end;
+ uint64_t scale = 1;
+
+ /* XXX Should we also parse things like '1.5M'? */
+ /* XXX Should we allow hex? If so, hex cannot use scaling suffixes,
+ * because some of them are valid hex digits.
+ */
+ errno = 0;
+ size = strtoimax (str, &end, 10);
+ if (str == end) {
+ *error = "could not parse size string";
+ *pstr = str;
+ return -1;
+ }
+ if (size < 0) {
+ *error = "size cannot be negative";
+ *pstr = str;
+ return -1;
+ }
+ if (errno) {
+ *error = "size exceeds maximum value";
+ *pstr = str;
+ return -1;
+ }
+
+ switch (*end) {
+ /* No suffix */
+ case '\0':
+ end--; /* Safe, since we already filtered out empty string */
+ break;
+
+ /* Powers of 1024 */
+ case 'e': case 'E':
+ scale *= 1024;
+ /* fallthru */
+ case 'p': case 'P':
+ scale *= 1024;
+ /* fallthru */
+ case 't': case 'T':
+ scale *= 1024;
+ /* fallthru */
+ case 'g': case 'G':
+ scale *= 1024;
+ /* fallthru */
+ case 'm': case 'M':
+ scale *= 1024;
+ /* fallthru */
+ case 'k': case 'K':
+ scale *= 1024;
+ /* fallthru */
+ case 'b': case 'B':
+ break;
+
+ /* "sectors", ie. units of 512 bytes, even if that's not the
real
+ * sector size
+ */
+ case 's': case 'S':
+ scale = 512;
+ break;
+
+ default:
+ *error = "could not parse size: unknown suffix";
+ *pstr = end;
+ return -1;
+ }
+
+ /* XXX Maybe we should support 'MiB' as a synonym for 'M';
and 'MB'
+ * for powers of 1000, for similarity to GNU tools. But for now,
+ * anything beyond 'M' is dropped.
+ */
+ if (end[1]) {
+ *error = "could not parse size: unknown suffix";
+ *pstr = end;
+ return -1;
+ }
+
+ if (INT64_MAX / scale < size) {
+ *error = "could not parse size: size * scale overflows";
+ *pstr = str;
+ return -1;
+ }
+
+ return size * scale;
+}
+
+#endif /* NBDKIT_HUMAN_SIZE_H */
diff --git a/common/include/test-human-size.c b/common/include/test-human-size.c
new file mode 100644
index 000000000..e8cbe7f41
--- /dev/null
+++ b/common/include/test-human-size.c
@@ -0,0 +1,133 @@
+/* nbdkit
+ * Copyright Red Hat
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * * Neither the name of Red Hat nor the names of its contributors may be
+ * used to endorse or promote products derived from this software without
+ * specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS
IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
+ * PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+ * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdbool.h>
+#include <stdint.h>
+
+#include "array-size.h"
+#include "human-size.h"
+
+int
+main (void)
+{
+ size_t i;
+ bool pass = true;
+ struct pair {
+ const char *str;
+ int64_t res;
+ } pairs[] = {
+ /* Bogus strings */
+ { "", -1 },
+ { "0x0", -1 },
+ { "garbage", -1 },
+ { "0garbage", -1 },
+ { "8E", -1 },
+ { "8192P", -1 },
+
+ /* Strings leading to overflow */
+ { "9223372036854775808", -1 }, /* INT_MAX + 1 */
+ { "18446744073709551614", -1 }, /* UINT64_MAX - 1 */
+ { "18446744073709551615", -1 }, /* UINT64_MAX */
+ { "18446744073709551616", -1 }, /* UINT64_MAX + 1 */
+ { "999999999999999999999999", -1 },
+
+ /* Strings representing negative values */
+ { "-1", -1 },
+ { "-2", -1 },
+ { "-9223372036854775809", -1 }, /* INT64_MIN - 1 */
+ { "-9223372036854775808", -1 }, /* INT64_MIN */
+ { "-9223372036854775807", -1 }, /* INT64_MIN + 1 */
+ { "-18446744073709551616", -1 }, /* -UINT64_MAX - 1 */
+ { "-18446744073709551615", -1 }, /* -UINT64_MAX */
+ { "-18446744073709551614", -1 }, /* -UINT64_MAX + 1 */
+
+ /* Strings we may want to support in the future */
+ { "M", -1 },
+ { "1MB", -1 },
+ { "1MiB", -1 },
+ { "1.5M", -1 },
+
+ /* Valid strings */
+ { "-0", 0 },
+ { "0", 0 },
+ { "+0", 0 },
+ { " 08", 8 },
+ { "1", 1 },
+ { "+1", 1 },
+ { "1234567890", 1234567890 },
+ { "+1234567890", 1234567890 },
+ { "9223372036854775807", INT64_MAX },
+ { "1s", 512 },
+ { "2S", 1024 },
+ { "1b", 1 },
+ { "1B", 1 },
+ { "1k", 1024 },
+ { "1K", 1024 },
+ { "1m", 1024 * 1024 },
+ { "1M", 1024 * 1024 },
+ { "+1M", 1024 * 1024 },
+ { "1g", 1024 * 1024 * 1024 },
+ { "1G", 1024 * 1024 * 1024 },
+ { "1t", 1024LL * 1024 * 1024 * 1024 },
+ { "1T", 1024LL * 1024 * 1024 * 1024 },
+ { "1p", 1024LL * 1024 * 1024 * 1024 * 1024 },
+ { "1P", 1024LL * 1024 * 1024 * 1024 * 1024 },
+ { "8191p", 1024LL * 1024 * 1024 * 1024 * 1024 * 8191 },
+ { "1e", 1024LL * 1024 * 1024 * 1024 * 1024 * 1024 },
+ { "1E", 1024LL * 1024 * 1024 * 1024 * 1024 * 1024 },
+ };
+
+ for (i = 0; i < ARRAY_SIZE (pairs); i++) {
+ const char *error = NULL, *pstr = NULL;
+ int64_t r;
+
+ r = human_size_parse (pairs[i].str, &error, &pstr);
+ if (r != pairs[i].res) {
+ fprintf (stderr,
+ "Wrong parse for %s, got %" PRId64 ", expected
%" PRId64 "\n",
+ pairs[i].str, r, pairs[i].res);
+ pass = false;
+ }
+ if (r == -1) {
+ if (error == NULL || pstr == NULL) {
+ fprintf (stderr, "Wrong error message handling for %s\n",
pairs[i].str);
+ pass = false;
+ }
+ }
+ }
+
+ exit (pass ? EXIT_SUCCESS : EXIT_FAILURE);
+}
diff --git a/server/public.c b/server/public.c
index 705ac3a47..a1ba603d4 100644
--- a/server/public.c
+++ b/server/public.c
@@ -76,6 +76,7 @@
#include "ascii-string.h"
#include "get_current_dir_name.h"
#include "getline.h"
+#include "human-size.h"
#include "poll.h"
#include "realpath.h"
#include "strndup.h"
@@ -343,83 +344,16 @@ nbdkit_parse_uint64_t (const char *what, const char *str,
uint64_t *rp)
NBDKIT_DLL_PUBLIC int64_t
nbdkit_parse_size (const char *str)
{
+ const char *error, *pstr;
int64_t size;
- char *end;
- uint64_t scale = 1;
- /* Disk sizes cannot usefully exceed off_t (which is signed) and
- * cannot be negative. */
- /* XXX Should we also parse things like '1.5M'? */
- /* XXX Should we allow hex? If so, hex cannot use scaling suffixes,
- * because some of them are valid hex digits */
- errno = 0;
- size = strtoimax (str, &end, 10);
- if (str == end) {
- nbdkit_error ("could not parse size string (%s)", str);
- return -1;
- }
- if (size < 0) {
- nbdkit_error ("size cannot be negative (%s)", str);
- return -1;
- }
- if (errno) {
- nbdkit_error ("size (%s) exceeds maximum value", str);
- return -1;
- }
-
- switch (*end) {
- /* No suffix */
- case '\0':
- end--; /* Safe, since we already filtered out empty string */
- break;
-
- /* Powers of 1024 */
- case 'e': case 'E':
- scale *= 1024;
- /* fallthru */
- case 'p': case 'P':
- scale *= 1024;
- /* fallthru */
- case 't': case 'T':
- scale *= 1024;
- /* fallthru */
- case 'g': case 'G':
- scale *= 1024;
- /* fallthru */
- case 'm': case 'M':
- scale *= 1024;
- /* fallthru */
- case 'k': case 'K':
- scale *= 1024;
- /* fallthru */
- case 'b': case 'B':
- break;
-
- /* "sectors", ie. units of 512 bytes, even if that's not the
real
- * sector size */
- case 's': case 'S':
- scale = 512;
- break;
-
- default:
- nbdkit_error ("could not parse size: unknown suffix
'%s'", end);
- return -1;
- }
-
- /* XXX Maybe we should support 'MiB' as a synonym for 'M';
and 'MB'
- * for powers of 1000, for similarity to GNU tools. But for now,
- * anything beyond 'M' is dropped. */
- if (end[1]) {
- nbdkit_error ("could not parse size: unknown suffix
'%s'", end);
- return -1;
- }
-
- if (INT64_MAX / scale < size) {
- nbdkit_error ("overflow computing size (%s)", str);
+ size = human_size_parse (str, &error, &pstr);
+ if (size == -1) {
+ nbdkit_error ("%s: %s", error, pstr);
return -1;
}
- return size * scale;
+ return size;
}
NBDKIT_DLL_PUBLIC int
diff --git a/.gitignore b/.gitignore
index 49af3998f..04fdcd723 100644
--- a/.gitignore
+++ b/.gitignore
@@ -40,6 +40,7 @@ plugins/*/*.3
/common/include/test-ascii-string
/common/include/test-byte-swapping
/common/include/test-checked-overflow
+/common/include/test-human-size
/common/include/test-isaligned
/common/include/test-ispowerof2
/common/include/test-iszero
--
2.41.0
Laszlo Ersek
2023-Sep-04 07:52 UTC
[Libguestfs] [PATCH nbdkit] server: Move size parsing code (nbdkit_parse_size) to common/include
On 9/3/23 17:22, Richard W.M. Jones wrote:> This is broadly simple code motion, intended so that we can reuse the > same code in libnbd. > --- > common/include/Makefile.am | 6 ++ > common/include/human-size.h | 137 +++++++++++++++++++++++++++++++ > common/include/test-human-size.c | 133 ++++++++++++++++++++++++++++++ > server/public.c | 78 ++---------------- > .gitignore | 1 + > 5 files changed, 283 insertions(+), 72 deletions(-) > > diff --git a/common/include/Makefile.am b/common/include/Makefile.am > index 3162e92c2..8e4de04f3 100644 > --- a/common/include/Makefile.am > +++ b/common/include/Makefile.am > @@ -42,6 +42,7 @@ EXTRA_DIST = \ > checked-overflow.h \ > compiler-macros.h \ > hexdigit.h \ > + human-size.h \ > isaligned.h \ > ispowerof2.h \ > iszero.h \ > @@ -63,6 +64,7 @@ TESTS = \ > test-ascii-string \ > test-byte-swapping \ > test-checked-overflow \ > + test-human-size \ > test-isaligned \ > test-ispowerof2 \ > test-iszero \ > @@ -93,6 +95,10 @@ test_checked_overflow_SOURCES = test-checked-overflow.c checked-overflow.h > test_checked_overflow_CPPFLAGS = -I$(srcdir) > test_checked_overflow_CFLAGS = $(WARNINGS_CFLAGS) > > +test_human_size_SOURCES = test-human-size.c human-size.h > +test_human_size_CPPFLAGS = -I$(srcdir) > +test_human_size_CFLAGS = $(WARNINGS_CFLAGS) > + > test_isaligned_SOURCES = test-isaligned.c isaligned.h > test_isaligned_CPPFLAGS = -I$(srcdir) > test_isaligned_CFLAGS = $(WARNINGS_CFLAGS) > diff --git a/common/include/human-size.h b/common/include/human-size.h > new file mode 100644 > index 000000000..788dbd7ba > --- /dev/null > +++ b/common/include/human-size.h > @@ -0,0 +1,137 @@ > +/* nbdkit > + * Copyright Red Hat > + * > + * Redistribution and use in source and binary forms, with or without > + * modification, are permitted provided that the following conditions are > + * met: > + * > + * * Redistributions of source code must retain the above copyright > + * notice, this list of conditions and the following disclaimer. > + * > + * * Redistributions in binary form must reproduce the above copyright > + * notice, this list of conditions and the following disclaimer in the > + * documentation and/or other materials provided with the distribution. > + * > + * * Neither the name of Red Hat nor the names of its contributors may be > + * used to endorse or promote products derived from this software without > + * specific prior written permission. > + * > + * THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS IS'' AND > + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, > + * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A > + * PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR > + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, > + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT > + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF > + * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND > + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, > + * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT > + * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF > + * SUCH DAMAGE. > + */ > + > +#ifndef NBDKIT_HUMAN_SIZE_H > +#define NBDKIT_HUMAN_SIZE_H > + > +#include <stdint.h> > +#include <inttypes.h> > +#include <errno.h> > + > +/* Attempt to parse a string with a possible scaling suffix, such as > + * "2M". Disk sizes cannot usefully exceed off_t (which is signed) > + * and cannot be negative. > + * > + * On error, returns -1 and sets *error and *pstr. You can form a > + * final error message by appending "<error>: <pstr>". > + */ > +static inline int64_t > +human_size_parse (const char *str, > + const char **error, const char **pstr) > +{ > + int64_t size; > + char *end; > + uint64_t scale = 1; > + > + /* XXX Should we also parse things like '1.5M'? */ > + /* XXX Should we allow hex? If so, hex cannot use scaling suffixes, > + * because some of them are valid hex digits. > + */ > + errno = 0; > + size = strtoimax (str, &end, 10);(1) A further improvement here (likely best done in a separate patch) could be to change the type of "size" to "intmax_t", from "int64_t". That way, the assignment will be safe even theoretically, *and* the overflow check at the bottom of the function (with the division & comparison of the quotient against INT_MAX) will work just the same.> + if (str == end) { > + *error = "could not parse size string"; > + *pstr = str; > + return -1; > + } > + if (size < 0) { > + *error = "size cannot be negative"; > + *pstr = str; > + return -1; > + } > + if (errno) { > + *error = "size exceeds maximum value"; > + *pstr = str; > + return -1; > + } > + > + switch (*end) { > + /* No suffix */ > + case '\0': > + end--; /* Safe, since we already filtered out empty string */ > + break; > + > + /* Powers of 1024 */ > + case 'e': case 'E': > + scale *= 1024; > + /* fallthru */ > + case 'p': case 'P': > + scale *= 1024; > + /* fallthru */ > + case 't': case 'T': > + scale *= 1024; > + /* fallthru */ > + case 'g': case 'G': > + scale *= 1024; > + /* fallthru */ > + case 'm': case 'M': > + scale *= 1024; > + /* fallthru */ > + case 'k': case 'K': > + scale *= 1024; > + /* fallthru */ > + case 'b': case 'B': > + break; > + > + /* "sectors", ie. units of 512 bytes, even if that's not the real > + * sector size > + */ > + case 's': case 'S': > + scale = 512; > + break; > + > + default: > + *error = "could not parse size: unknown suffix"; > + *pstr = end; > + return -1; > + } > + > + /* XXX Maybe we should support 'MiB' as a synonym for 'M'; and 'MB' > + * for powers of 1000, for similarity to GNU tools. But for now, > + * anything beyond 'M' is dropped. > + */ > + if (end[1]) { > + *error = "could not parse size: unknown suffix"; > + *pstr = end; > + return -1; > + } > + > + if (INT64_MAX / scale < size) { > + *error = "could not parse size: size * scale overflows"; > + *pstr = str; > + return -1; > + } > + > + return size * scale; > +} > + > +#endif /* NBDKIT_HUMAN_SIZE_H */ > diff --git a/common/include/test-human-size.c b/common/include/test-human-size.c > new file mode 100644 > index 000000000..e8cbe7f41 > --- /dev/null > +++ b/common/include/test-human-size.c > @@ -0,0 +1,133 @@ > +/* nbdkit > + * Copyright Red Hat > + * > + * Redistribution and use in source and binary forms, with or without > + * modification, are permitted provided that the following conditions are > + * met: > + * > + * * Redistributions of source code must retain the above copyright > + * notice, this list of conditions and the following disclaimer. > + * > + * * Redistributions in binary form must reproduce the above copyright > + * notice, this list of conditions and the following disclaimer in the > + * documentation and/or other materials provided with the distribution. > + * > + * * Neither the name of Red Hat nor the names of its contributors may be > + * used to endorse or promote products derived from this software without > + * specific prior written permission. > + * > + * THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS IS'' AND > + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, > + * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A > + * PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR > + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, > + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT > + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF > + * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND > + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, > + * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT > + * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF > + * SUCH DAMAGE. > + */ > + > +#include <config.h> > + > +#include <stdio.h> > +#include <stdlib.h> > +#include <stdbool.h> > +#include <stdint.h> > + > +#include "array-size.h" > +#include "human-size.h" > + > +int > +main (void) > +{ > + size_t i; > + bool pass = true; > + struct pair { > + const char *str; > + int64_t res; > + } pairs[] = { > + /* Bogus strings */ > + { "", -1 }, > + { "0x0", -1 }, > + { "garbage", -1 }, > + { "0garbage", -1 }, > + { "8E", -1 }, > + { "8192P", -1 }, > + > + /* Strings leading to overflow */ > + { "9223372036854775808", -1 }, /* INT_MAX + 1 */ > + { "18446744073709551614", -1 }, /* UINT64_MAX - 1 */ > + { "18446744073709551615", -1 }, /* UINT64_MAX */ > + { "18446744073709551616", -1 }, /* UINT64_MAX + 1 */ > + { "999999999999999999999999", -1 }, > + > + /* Strings representing negative values */ > + { "-1", -1 }, > + { "-2", -1 }, > + { "-9223372036854775809", -1 }, /* INT64_MIN - 1 */ > + { "-9223372036854775808", -1 }, /* INT64_MIN */ > + { "-9223372036854775807", -1 }, /* INT64_MIN + 1 */ > + { "-18446744073709551616", -1 }, /* -UINT64_MAX - 1 */ > + { "-18446744073709551615", -1 }, /* -UINT64_MAX */ > + { "-18446744073709551614", -1 }, /* -UINT64_MAX + 1 */ > + > + /* Strings we may want to support in the future */ > + { "M", -1 }, > + { "1MB", -1 }, > + { "1MiB", -1 }, > + { "1.5M", -1 }, > + > + /* Valid strings */ > + { "-0", 0 }, > + { "0", 0 }, > + { "+0", 0 }, > + { " 08", 8 }, > + { "1", 1 }, > + { "+1", 1 }, > + { "1234567890", 1234567890 }, > + { "+1234567890", 1234567890 }, > + { "9223372036854775807", INT64_MAX }, > + { "1s", 512 }, > + { "2S", 1024 }, > + { "1b", 1 }, > + { "1B", 1 }, > + { "1k", 1024 }, > + { "1K", 1024 }, > + { "1m", 1024 * 1024 }, > + { "1M", 1024 * 1024 }, > + { "+1M", 1024 * 1024 }, > + { "1g", 1024 * 1024 * 1024 }, > + { "1G", 1024 * 1024 * 1024 }, > + { "1t", 1024LL * 1024 * 1024 * 1024 }, > + { "1T", 1024LL * 1024 * 1024 * 1024 }, > + { "1p", 1024LL * 1024 * 1024 * 1024 * 1024 }, > + { "1P", 1024LL * 1024 * 1024 * 1024 * 1024 }, > + { "8191p", 1024LL * 1024 * 1024 * 1024 * 1024 * 8191 }, > + { "1e", 1024LL * 1024 * 1024 * 1024 * 1024 * 1024 }, > + { "1E", 1024LL * 1024 * 1024 * 1024 * 1024 * 1024 }, > + }; > + > + for (i = 0; i < ARRAY_SIZE (pairs); i++) { > + const char *error = NULL, *pstr = NULL; > + int64_t r; > + > + r = human_size_parse (pairs[i].str, &error, &pstr); > + if (r != pairs[i].res) { > + fprintf (stderr, > + "Wrong parse for %s, got %" PRId64 ", expected %" PRId64 "\n", > + pairs[i].str, r, pairs[i].res); > + pass = false; > + } > + if (r == -1) { > + if (error == NULL || pstr == NULL) { > + fprintf (stderr, "Wrong error message handling for %s\n", pairs[i].str); > + pass = false; > + } > + } > + } > + > + exit (pass ? EXIT_SUCCESS : EXIT_FAILURE); > +}(2) I don't like that we're repeating the test cases here, from test_nbdkit_parse_size() [server/test-public.c]. Originally I intended to ask "why not just *move* that code as well", but I think I see the point... Namely, in test_nbdkit_parse_size(), we still need to test nbdkit_error() -- via "error_flagged" --, and nbdkit_error() remains unique to test_nbdkit_parse_size(), after factoring out human_size_parse(). And so, for triggering the errors, we need to keep the same test cases. ... Would it be possible to move the "pairs" array into a separate C file under "common"? (Not necessarily under "common/include".) We'd need a new header file (for defining the "pair" type, for declaring the "pairs" array, and for declaring the "num_pairs" constant, which would have to be a global variable then.) If that's too difficult or intrusive, then please at least cross-reference each source file from the other, in new comments, so that whenever we update one of them, we don't forget the other. (3) Calling "exit" at the end is a bit awkward to me. Correct, but "return" would work just as fine. With the cross-refs added: Reviewed-by: Laszlo Ersek <lersek at redhat.com> Laszlo> diff --git a/server/public.c b/server/public.c > index 705ac3a47..a1ba603d4 100644 > --- a/server/public.c > +++ b/server/public.c > @@ -76,6 +76,7 @@ > #include "ascii-string.h" > #include "get_current_dir_name.h" > #include "getline.h" > +#include "human-size.h" > #include "poll.h" > #include "realpath.h" > #include "strndup.h" > @@ -343,83 +344,16 @@ nbdkit_parse_uint64_t (const char *what, const char *str, uint64_t *rp) > NBDKIT_DLL_PUBLIC int64_t > nbdkit_parse_size (const char *str) > { > + const char *error, *pstr; > int64_t size; > - char *end; > - uint64_t scale = 1; > > - /* Disk sizes cannot usefully exceed off_t (which is signed) and > - * cannot be negative. */ > - /* XXX Should we also parse things like '1.5M'? */ > - /* XXX Should we allow hex? If so, hex cannot use scaling suffixes, > - * because some of them are valid hex digits */ > - errno = 0; > - size = strtoimax (str, &end, 10); > - if (str == end) { > - nbdkit_error ("could not parse size string (%s)", str); > - return -1; > - } > - if (size < 0) { > - nbdkit_error ("size cannot be negative (%s)", str); > - return -1; > - } > - if (errno) { > - nbdkit_error ("size (%s) exceeds maximum value", str); > - return -1; > - } > - > - switch (*end) { > - /* No suffix */ > - case '\0': > - end--; /* Safe, since we already filtered out empty string */ > - break; > - > - /* Powers of 1024 */ > - case 'e': case 'E': > - scale *= 1024; > - /* fallthru */ > - case 'p': case 'P': > - scale *= 1024; > - /* fallthru */ > - case 't': case 'T': > - scale *= 1024; > - /* fallthru */ > - case 'g': case 'G': > - scale *= 1024; > - /* fallthru */ > - case 'm': case 'M': > - scale *= 1024; > - /* fallthru */ > - case 'k': case 'K': > - scale *= 1024; > - /* fallthru */ > - case 'b': case 'B': > - break; > - > - /* "sectors", ie. units of 512 bytes, even if that's not the real > - * sector size */ > - case 's': case 'S': > - scale = 512; > - break; > - > - default: > - nbdkit_error ("could not parse size: unknown suffix '%s'", end); > - return -1; > - } > - > - /* XXX Maybe we should support 'MiB' as a synonym for 'M'; and 'MB' > - * for powers of 1000, for similarity to GNU tools. But for now, > - * anything beyond 'M' is dropped. */ > - if (end[1]) { > - nbdkit_error ("could not parse size: unknown suffix '%s'", end); > - return -1; > - } > - > - if (INT64_MAX / scale < size) { > - nbdkit_error ("overflow computing size (%s)", str); > + size = human_size_parse (str, &error, &pstr); > + if (size == -1) { > + nbdkit_error ("%s: %s", error, pstr); > return -1; > } > > - return size * scale; > + return size; > } > > NBDKIT_DLL_PUBLIC int > diff --git a/.gitignore b/.gitignore > index 49af3998f..04fdcd723 100644 > --- a/.gitignore > +++ b/.gitignore > @@ -40,6 +40,7 @@ plugins/*/*.3 > /common/include/test-ascii-string > /common/include/test-byte-swapping > /common/include/test-checked-overflow > +/common/include/test-human-size > /common/include/test-isaligned > /common/include/test-ispowerof2 > /common/include/test-iszero