[RFC nbdkit PATCH] utils: Revamp nbdkit_parse_size
by Eric Blake
The existing implementation admitted that it was not very robust
in the face of garbage input; tighten things up by reimplementing
the function, and add testsuite coverage to ensure further tweaks
do not break things.
The testsuite additions were interesting - we didn't have any easy
way to link against a subset of the src/ files (all previous uses
of the util functions have been through dynamic .so libraries,
rather than standalone binaries).
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
RFC, because I don't know if I like my testsuite additions; automake
in particular was rather noisy about it:
tests/Makefile.am:99: warning: source file '$(top_srcdir)/src/utils.c' is in a subdirectory,
tests/Makefile.am:99: but option 'subdir-objects' is disabled
automake-1.15: warning: possible forward-incompatibility.
automake-1.15: At least a source file is in a subdirectory, but the 'subdir-objects'
automake-1.15: automake option hasn't been enabled. For now, the corresponding output
automake-1.15: object file(s) will be placed in the top-level directory. However,
automake-1.15: this behaviour will change in future Automake versions: they will
automake-1.15: unconditionally cause object files to be placed in the same subdirectory
automake-1.15: of the corresponding sources.
automake-1.15: You are advised to start using 'subdir-objects' option throughout your
automake-1.15: project, to avoid future incompatibilities.
Maybe there's a smarter way to do this?
Or, I could just commit the src/utils.c change, and leave out the
testsuite additions.
---
src/utils.c | 95 ++++++++++++++++++++++++++---------------
tests/Makefile.am | 11 +++++
tests/test-utils.c | 121 +++++++++++++++++++++++++++++++++++++++++++++++++++++
.gitignore | 1 +
4 files changed, 195 insertions(+), 33 deletions(-)
create mode 100644 tests/test-utils.c
diff --git a/src/utils.c b/src/utils.c
index 5663043..6e04bc8 100644
--- a/src/utils.c
+++ b/src/utils.c
@@ -1,5 +1,5 @@
/* nbdkit
- * Copyright (C) 2013 Red Hat Inc.
+ * Copyright (C) 2013-2018 Red Hat Inc.
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
@@ -80,49 +80,78 @@ nbdkit_absolute_path (const char *path)
return ret;
}
-/* XXX Multiple problems with this function. Really we should use the
- * 'human*' functions from gnulib.
+/* Parse a string with possible scaling suffix, or return -1 after reporting
+ * the error.
*/
int64_t
nbdkit_parse_size (const char *str)
{
uint64_t size;
- char t;
+ char *end;
+ uint64_t scale = 1;
- if (sscanf (str, "%" SCNu64 "%c", &size, &t) == 2) {
- switch (t) {
- case 'b': case 'B':
- return (int64_t) size;
- case 'k': case 'K':
- return (int64_t) size * 1024;
- case 'm': case 'M':
- return (int64_t) size * 1024 * 1024;
- case 'g': case 'G':
- return (int64_t) size * 1024 * 1024 * 1024;
- case 't': case 'T':
- return (int64_t) size * 1024 * 1024 * 1024 * 1024;
- case 'p': case 'P':
- return (int64_t) size * 1024 * 1024 * 1024 * 1024 * 1024;
- case 'e': case 'E':
- return (int64_t) size * 1024 * 1024 * 1024 * 1024 * 1024 * 1024;
+ /* Scan unsigned, but range check later that result fits in signed. */
+ /* 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 = strtoumax (str, &end, 10);
+ if (errno || str == end) {
+ nbdkit_error ("could not parse size string (%s)", str);
+ return -1;
+ }
+ switch (*end) {
+ 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;
- case 's': case 'S': /* "sectors", ie. units of 512 bytes,
- * even if that's not the real sector size
- */
- return (int64_t) size * 512;
+ case 's': case 'S': /* "sectors", ie. units of 512 bytes,
+ * even if that's not the real sector size
+ */
+ scale = 512;
+ break;
- default:
- nbdkit_error ("could not parse size: unknown specifier '%c'", t);
- return -1;
- }
+ default:
+ nbdkit_error ("could not parse size: unknown suffix '%s'", end);
+ return -1;
}
- /* bytes */
- if (sscanf (str, "%" SCNu64, &size) == 1)
- return (int64_t) size;
+ /* XXX Maybe we should support 'MiB' as 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);
+ return -1;
+ }
- nbdkit_error ("could not parse size string (%s)", str);
- return -1;
+ return size * scale;
}
/* Read a password from configuration value. */
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 1e32cb6..baae6a1 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -90,8 +90,19 @@ TESTS_ENVIRONMENT = PATH=$(abs_top_builddir):$(PATH)
TESTS = \
test-help.sh \
test-version.sh \
+ test-utils \
test-dump-config.sh
+check_PROGRAMS += \
+ test-utils
+
+test_utils_SOURCES = test-utils.c \
+ $(top_srcdir)/src/utils.c \
+ $(top_srcdir)/src/cleanup.c \
+ $(top_srcdir)/include/nbdkit-plugin.h
+test_utils_CPPFLAGS = -I$(top_srcdir)/include
+test_utils_CFLAGS = $(WARNINGS_CFLAGS)
+
if HAVE_PLUGINS
TESTS += \
diff --git a/tests/test-utils.c b/tests/test-utils.c
new file mode 100644
index 0000000..af08416
--- /dev/null
+++ b/tests/test-utils.c
@@ -0,0 +1,121 @@
+/* nbdkit
+ * Copyright (C) 2018 Red Hat Inc.
+ * All rights reserved.
+ *
+ * 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 <inttypes.h>
+#include <stdbool.h>
+
+#include <nbdkit-plugin.h>
+
+static bool error_flagged;
+
+/* Stub for linking against minimal source files */
+void
+nbdkit_error (const char *fs, ...)
+{
+ error_flagged = true;
+}
+
+static bool
+test_nbdkit_parse_size (void)
+{
+ bool pass = true;
+ struct pair {
+ const char *str;
+ int64_t res;
+ } pairs[] = {
+ /* Bogus strings */
+ { "", -1 },
+ { "0x0", -1 },
+ { "garbage", -1 },
+ { "0garbage", -1 },
+ { "M", -1 },
+ { "-1", -1 },
+ { "-2", -1 },
+ { "9223372036854775808", -1 },
+ { "-9223372036854775808", -1 },
+ { "8E", -1 },
+ { "8192P", -1 },
+
+ /* Valid strings */
+ { "0", 0 },
+ { " 08", 8 },
+ { "9223372036854775807", INT64_MAX },
+ { "1s", 512 },
+ { "2S", 1024 },
+ { "1b", 1 },
+ { "1B", 1 },
+ { "1k", 1024 },
+ { "1K", 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 },
+ { "1e", 1024LL * 1024 * 1024 * 1024 * 1024 * 1024 },
+ { "1E", 1024LL * 1024 * 1024 * 1024 * 1024 * 1024 },
+ };
+
+ for (size_t i = 0; i < sizeof pairs / sizeof pairs[0]; i++) {
+ int64_t r;
+
+ error_flagged = false;
+ r = nbdkit_parse_size (pairs[i].str);
+ 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) != error_flagged) {
+ fprintf (stderr, "Wrong error message handling for %s\n", pairs[i].str);
+ pass = false;
+ }
+ }
+
+ return pass;
+}
+
+int
+main (int argc, char *argv[])
+{
+ bool pass = true;
+ pass &= test_nbdkit_parse_size ();
+ /* XXX tests nbdkit_absolute_path, nbdkit_read_password */
+ return pass ? 0 : 1;
+}
diff --git a/.gitignore b/.gitignore
index 09ed262..28215bc 100644
--- a/.gitignore
+++ b/.gitignore
@@ -72,5 +72,6 @@ Makefile.in
/tests/test-socket-activation
/tests/test-split
/tests/test-streaming
+/tests/test-utils
/tests/test-xz
/test-driver
--
2.14.3
6 years, 10 months
- 1
- 0
AM_GNU_GETTEXT failing on fedora
by Marc Pawlowsky
I am trying to build from source but am getting errors
-bash-4.3$ grep -n AM_GNU_GET /tmp/t
2595:m4/guestfs_libraries.m4:145: warning: macro 'AM_GNU_GETTEXT' not
found in library
2598:m4/guestfs_libraries.m4:145: warning: macro 'AM_GNU_GETTEXT' not
found in library
2600:configure:57872: error: possibly undefined macro: AM_GNU_GETTEXT
3834:./configure: line 57872: `AM_GNU_GETTEXT(external)'
Dockerfile contents
FROM fedora:latest
RUN dnf update -y
RUN dnf -y install 'dnf-command(config-manager)'
RUN dnf config-manager --set-enabled 'updates-testing'
RUN dnf update -y
RUN dnf group install -y "C Development Tools and Libraries"
RUN dnf group install -y "Development Tools"
RUN dnf -y builddep libguestfs
RUN cd / && git clone https://github.com/libguestfs/libguestfs.git
RUN cd /libguestfs && ./autogen.sh
Same problem if I build from stable branch.
RUN cd / && git clone -b stable-1.36
https://github.com/libguestfs/libguestfs.git
Any hints?
6 years, 10 months
- 2
- 1
Having issues with virt-sparsify
by Simon Ruzgar
Im having an issue running “virt-sparsify”, getting the following error:
root@server:# virt-sparsify --check-tmpdir warn --compress ws1709.qcow2 ws1709core.qcow2
Input disk virtual size = 34359738368 bytes (32.0G)
Create overlay file in /tmp to protect source disk ...
Examine source disk ...
Fatal error: exception Guestfs.Error("/usr/bin/supermin-helper exited with error status 1.
To see full error messages you may need to enable debugging.
See http://libguestfs.org/guestfs-faq.1.html#debugging-libguestfs")
Debug output:
root@server: # libguestfs-test-tool
************************************************************
* IMPORTANT NOTICE
*
* When reporting bugs, include the COMPLETE, UNEDITED
* output below in your bug report.
*
************************************************************
PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
SELinux: sh: 1: getenforce: not found
guestfs_get_append: (null)
guestfs_get_backend: direct
guestfs_get_autosync: 1
guestfs_get_cachedir: /var/tmp
guestfs_get_direct: 0
guestfs_get_hv: /usr/bin/qemu-system-x86_64
guestfs_get_memsize: 500
guestfs_get_network: 0
guestfs_get_path: /usr/lib/guestfs
guestfs_get_pgroup: 0
guestfs_get_program: libguestfs-test-tool
guestfs_get_recovery_proc: 1
guestfs_get_selinux: 0
guestfs_get_smp: 1
guestfs_get_tmpdir: /tmp
guestfs_get_trace: 0
guestfs_get_verbose: 1
host_cpu: x86_64
Launching appliance, timeout set to 600 seconds.
libguestfs: launch: program=libguestfs-test-tool
libguestfs: launch: version=1.24.5
libguestfs: launch: backend registered: unix
libguestfs: launch: backend registered: uml
libguestfs: launch: backend registered: libvirt
libguestfs: launch: backend registered: direct
libguestfs: launch: backend=direct
libguestfs: launch: tmpdir=/tmp/libguestfsxEv0vt
libguestfs: launch: umask=0022
libguestfs: launch: euid=0
libguestfs: command: run: /usr/bin/supermin-helper
libguestfs: command: run: \ --verbose
libguestfs: command: run: \ -f checksum
libguestfs: command: run: \ --host-cpu x86_64
libguestfs: command: run: \ /usr/lib/guestfs/supermin.d
supermin helper [00000ms] whitelist = (not specified)
supermin helper [00000ms] host_cpu = x86_64
supermin helper [00000ms] dtb_wildcard = (not specified)
supermin helper [00000ms] inputs:
supermin helper [00000ms] inputs[0] = /usr/lib/guestfs/supermin.d
supermin helper [00000ms] outputs:
supermin helper [00000ms] kernel = (none)
supermin helper [00000ms] dtb = (none)
supermin helper [00000ms] initrd = (none)
supermin helper [00000ms] appliance = (none)
checking modpath /lib/modules/3.13.0-32-generic is a directory
checking modpath /lib/modules/3.19.0-68-generic is a directory
checking modpath /lib/modules/3.13.0-58-generic is a directory
picked kernel vmlinuz-3.19.0-68-generic
supermin helper [00000ms] finished creating kernel
supermin helper [00000ms] visiting /usr/lib/guestfs/supermin.d
supermin helper [00000ms] visiting /usr/lib/guestfs/supermin.d/daemon.img.gz
supermin helper [00000ms] visiting /usr/lib/guestfs/supermin.d/init.img
supermin helper [00000ms] visiting /usr/lib/guestfs/supermin.d/udev-rules.img
supermin helper [00000ms] adding kernel modules
supermin helper [00023ms] finished creating appliance
libguestfs: checksum of existing appliance: 8b61a04874843524a7b5c03f33b95dabf61f2fbdc20e633d59fc8d872c061678
libguestfs: [00024ms] begin building supermin appliance
libguestfs: [00024ms] run supermin-helper
libguestfs: command: run: /usr/bin/supermin-helper
libguestfs: command: run: \ --verbose
libguestfs: command: run: \ --copy-kernel
libguestfs: command: run: \ -f ext2
libguestfs: command: run: \ --host-cpu x86_64
libguestfs: command: run: \ /usr/lib/guestfs/supermin.d
libguestfs: command: run: \ --output-kernel /var/tmp/guestfs.jgjlPT/kernel
libguestfs: command: run: \ --output-initrd /var/tmp/guestfs.jgjlPT/initrd
libguestfs: command: run: \ --output-appliance /var/tmp/guestfs.jgjlPT/root
supermin helper [00000ms] whitelist = (not specified)
supermin helper [00000ms] host_cpu = x86_64
supermin helper [00000ms] dtb_wildcard = (not specified)
supermin helper [00000ms] inputs:
supermin helper [00000ms] inputs[0] = /usr/lib/guestfs/supermin.d
supermin helper [00000ms] outputs:
supermin helper [00000ms] kernel = /var/tmp/guestfs.jgjlPT/kernel
supermin helper [00000ms] dtb = (none)
supermin helper [00000ms] initrd = /var/tmp/guestfs.jgjlPT/initrd
supermin helper [00000ms] appliance = /var/tmp/guestfs.jgjlPT/root
checking modpath /lib/modules/3.13.0-32-generic is a directory
checking modpath /lib/modules/3.19.0-68-generic is a directory
checking modpath /lib/modules/3.13.0-58-generic is a directory
picked kernel vmlinuz-3.19.0-68-generic
supermin helper [00006ms] finished creating kernel
supermin helper [00224ms] finished mke2fs
supermin helper [00224ms] visiting /usr/lib/guestfs/supermin.d
supermin helper [00224ms] visiting /usr/lib/guestfs/supermin.d/daemon.img.gz
supermin helper [00240ms] visiting /usr/lib/guestfs/supermin.d/init.img
supermin helper [00241ms] visiting /usr/lib/guestfs/supermin.d/udev-rules.img
/usr/bin/supermin-helper: ext2: parent directory not found: /lib: File not found by ext2_lookup
libguestfs: error: /usr/bin/supermin-helper exited with error status 1, see debug messages above
libguestfs: command: run: rm
libguestfs: command: run: \ -rf /var/tmp/guestfs.jgjlPT
libguestfs-test-tool: failed to launch appliance
libguestfs: closing guestfs handle 0xed7150 (state 0)
libguestfs: command: run: rm
libguestfs: command: run: \ -rf /tmp/libguestfsxEv0vt
Many thanks
Simon
This e-mail message and any attachments to it are intended only for the named recipients and may contain legally privileged and/or confidential information. If you are not one of the intended recipients, do not duplicate or forward this e-mail message.
6 years, 10 months
- 2
- 2