Libguestfs - Sep 2021 - [PATCH nbdinfo v2 2/3] info: Add human size to ordinary output

For example 1024 is returned as "1K".

This does not attempt to handle decimals or SI units.  If the number
isn't some multiple of a power of 1024 then it is returned as bytes (a
flag is available to indicate this).

I looked at both the gnulib and qemu versions of this function.  The
gnulib version is not under a license which is compatible with libnbd
and is also really complicated, although it does handle fractions and
SI units.  The qemu version is essentially just frexp + sprintf and
doesn't attempt to convert to the human-readable version reversibly.
---
 .gitignore                     |  1 +
 common/utils/Makefile.am       | 10 +++-
 common/utils/human-size.c      | 54 +++++++++++++++++++++
 common/utils/human-size.h      | 49 +++++++++++++++++++
 common/utils/test-human-size.c | 89 ++++++++++++++++++++++++++++++++++
 5 files changed, 201 insertions(+), 2 deletions(-)

diff --git a/.gitignore b/.gitignore
index 2aa1fd99..5fc59677 100644
--- a/.gitignore
+++ b/.gitignore
@@ -31,6 +31,7 @@ Makefile.in
 /bash/nbdcopy
 /bash/nbdfuse
 /bash/nbdinfo
+/common/utils/test-human-size
 /common/utils/test-vector
 /compile
 /config.cache
diff --git a/common/utils/Makefile.am b/common/utils/Makefile.am
index 1ca4a370..b273ada1 100644
--- a/common/utils/Makefile.am
+++ b/common/utils/Makefile.am
@@ -34,6 +34,8 @@ include $(top_srcdir)/common-rules.mk
 noinst_LTLIBRARIES = libutils.la
 
 libutils_la_SOURCES = \
+	human-size.c \
+	human-size.h \
 	vector.c \
 	vector.h \
 	version.c \
@@ -50,8 +52,12 @@ libutils_la_LIBADD = \
 
 # Unit tests.
 
-TESTS = test-vector
-check_PROGRAMS = test-vector
+TESTS = test-human-size test-vector
+check_PROGRAMS = test-human-size test-vector
+
+test_human_size_SOURCES = test-human-size.c human-size.c human-size.h
+test_human_size_CPPFLAGS = -I$(srcdir)
+test_human_size_CFLAGS = $(WARNINGS_CFLAGS)
 
 test_vector_SOURCES = test-vector.c vector.c vector.h
 test_vector_CPPFLAGS = -I$(srcdir)
diff --git a/common/utils/human-size.c b/common/utils/human-size.c
new file mode 100644
index 00000000..772f2489
--- /dev/null
+++ b/common/utils/human-size.c
@@ -0,0 +1,54 @@
+/* nbd client library in userspace
+ * Copyright (C) 2020-2021 Red Hat Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <inttypes.h>
+
+#include "human-size.h"
+
+char *
+human_size (char *buf, uint64_t bytes, bool *human)
+{
+  static const char *ext[] = { "E", "P", "T",
"G", "M", "K", "" };
+  size_t i;
+
+  if (buf == NULL) {
+    buf = malloc (HUMAN_SIZE_LONGEST);
+    if (buf == NULL)
+      return NULL;
+  }
+
+  /* Work out which extension to use, if any. */
+  for (i = 6; i >= 0; --i) {
+    if (bytes == 0 || (bytes & 1023) != 0)
+      break;
+    bytes /= 1024;
+  }
+
+  /* Set to flag to true if we're going to add a human-readable extension.
*/
+  if (human)
+    *human = ext[i][0] != '\0';
+
+  snprintf (buf, HUMAN_SIZE_LONGEST, "%" PRIu64 "%s",
bytes, ext[i]);
+  return buf;
+}
diff --git a/common/utils/human-size.h b/common/utils/human-size.h
new file mode 100644
index 00000000..9ee78803
--- /dev/null
+++ b/common/utils/human-size.h
@@ -0,0 +1,49 @@
+/* nbd client library in userspace
+ * Copyright (C) 2020-2021 Red Hat Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#ifndef LIBNBD_HUMAN_SIZE_H
+#define LIBNBD_HUMAN_SIZE_H
+
+#include <stdbool.h>
+#include <stdint.h>
+
+/* If you allocate a buffer of at least this length in bytes and pass
+ * it as the first parameter to human_size, then it will not overrun.
+ */
+#define HUMAN_SIZE_LONGEST 64
+
+/* Convert bytes to a human-readable string.
+ *
+ * This is roughly the opposite of nbdkit_parse_size.  It will convert
+ * multiples of powers of 1024 to the appropriate human size with the
+ * right extension like 'M' or 'G'.  Anything that cannot be
converted
+ * is returned as bytes.  The *human flag is set to true if the output
+ * was abbreviated to a human-readable size, or false if it is just
+ * bytes.
+ *
+ * If buf == NULL, a buffer is allocated and returned.  In this case
+ * the returned buffer must be freed.
+ *
+ * buf may also be allocated by the caller, in which case it must be
+ * at least HUMAN_SIZE_LONGEST bytes.
+ *
+ * On error the function returns an error and sets errno.
+ */
+extern char *human_size (char *buf, uint64_t bytes, bool *human);
+
+#endif /* LIBNBD_HUMAN_SIZE_H */
diff --git a/common/utils/test-human-size.c b/common/utils/test-human-size.c
new file mode 100644
index 00000000..d35a21bf
--- /dev/null
+++ b/common/utils/test-human-size.c
@@ -0,0 +1,89 @@
+/* nbd client library in userspace
+ * Copyright (C) 2020-2021 Red Hat Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <inttypes.h>
+#include <string.h>
+
+#include "human-size.h"
+
+static unsigned errors = 0;
+
+static void
+test (uint64_t bytes, const char *expected, bool expected_human_flag)
+{
+  char actual[HUMAN_SIZE_LONGEST];
+  bool actual_human_flag;
+
+  human_size (actual, bytes, &actual_human_flag);
+
+  if (strcmp (actual, expected) == 0 ||
+      actual_human_flag != expected_human_flag) {
+    printf ("test-human-size: %" PRIu64 " -> \"%s\"
(%s) OK\n",
+            bytes, actual, actual_human_flag ? "true" :
"false");
+    fflush (stdout);
+  }
+  else {
+    fprintf (stderr,
+             "test-human-size: error: test case %" PRIu64
+             "expected \"%s\" (%s) "
+             "but returned \"%s\" (%s)\n",
+             bytes,
+             expected, expected_human_flag ? "true" :
"false",
+             actual, actual_human_flag ? "true" : "false");
+    errors++;
+  }
+}
+
+int
+main (int argc, char *argv[])
+{
+  test (0, "0", false);
+  test (1, "1", false);
+  test (512, "512", false);
+  test (1023, "1023", false);
+  test (1024, "1K", true);
+  test (1025, "1025", false);
+  test (2047, "2047", false);
+  test (2048, "2K", true);
+  test (3 * 1024, "3K", true);
+
+  test (1023 * 1024, "1023K", true);
+  test (1048575, "1048575", false);
+  test (1048576, "1M", true);
+  test (1048577, "1048577", false);
+
+  test (UINT64_C(1073741824), "1G", true);
+
+  test (UINT64_C(1099511627776), "1T", true);
+  test (UINT64_C(1099511627777), "1099511627777", false);
+  test (UINT64_C(1099511627776) + 1024, "1073741825K", true);
+
+  test (UINT64_C(1125899906842624), "1P", true);
+
+  test ((uint64_t)INT64_MAX+1, "8E", true);
+  test (UINT64_MAX-1023, "18014398509481983K", true);
+  test (UINT64_MAX, "18446744073709551615", false);
+
+  exit (errors == 0 ? EXIT_SUCCESS : EXIT_FAILURE);
+}
-- 
2.32.0

For example:

  $ nbdkit null 1G --run 'nbdinfo "$uri"'
  protocol: newstyle-fixed without TLS
  export="":
  	export-size: 1073741824 (1G)

If the value cannot be abbreviated then the output doesn't include the
part in parentheses:

  $ nbdkit null 1023 --run 'nbdinfo "$uri"'
  protocol: newstyle-fixed without TLS
  export="":
  	export-size: 1023

The human-readable output changes, but callers shouldn't be parsing it
(use --size and/or --json instead).
---
 info/nbdinfo.pod |  9 +++++----
 info/show.c      | 13 +++++++++++--
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/info/nbdinfo.pod b/info/nbdinfo.pod
index fbd0ef20..f64fe213 100644
--- a/info/nbdinfo.pod
+++ b/info/nbdinfo.pod
@@ -29,7 +29,7 @@
L<https://github.com/NetworkBlockDevice/nbd/blob/master/doc/uri.md>):
  $ nbdinfo nbd://localhost
  protocol: newstyle-fixed without TLS
  export="":
-         export-size: 1048576
+         export-size: 1048576 (1M)
          content: data
          uri: nbd://localhost:10809/
          is_rotational: false
@@ -78,7 +78,8 @@ the I<--json> parameter:
        "block_size_minimum": 1,
        "block_size_preferred": 4096,
        "block_size_maximum": 33554432,
-       "export-size": 2125119488
+       "export-size": 2125119488,
+       "export-size-str": "2075312K"
      }
    ]
  }
@@ -222,11 +223,11 @@ For example:
  $ nbdkit file dir=. --run 'nbdinfo --list "$uri"'
  protocol: newstyle-fixed without TLS
  export="Fedora-Workstation-Live-x86_64-29-1.2.iso":
-     export-size: 1931476992
+     export-size: 1931476992 (1842M)
      uri: nbd://localhost:10809/Fedora-Workstation-Live-x86_64-29-1.2.iso
      [...]
  export="debian-10.4.0-amd64-DVD-1.iso":
-     export-size: 3955556352
+     export-size: 3955556352 (3862848K)
      uri: nbd://localhost:10809/debian-10.4.0-amd64-DVD-1.iso
      [...]
 
diff --git a/info/show.c b/info/show.c
index 893178cb..ff241a83 100644
--- a/info/show.c
+++ b/info/show.c
@@ -28,6 +28,7 @@
 
 #include <libnbd.h>
 
+#include "human-size.h"
 #include "vector.h"
 
 #include "nbdinfo.h"
@@ -45,6 +46,8 @@ show_one_export (struct nbd_handle *nbd, const char *desc,
                  bool first, bool last)
 {
   int64_t i, size;
+  char size_str[HUMAN_SIZE_LONGEST];
+  bool human_size_flag;
   char *export_name = NULL;
   char *export_desc = NULL;
   char *content = NULL;
@@ -75,6 +78,8 @@ show_one_export (struct nbd_handle *nbd, const char *desc,
     exit (EXIT_FAILURE);
   }
 
+  human_size (size_str, size, &human_size_flag);
+
   uri = nbd_get_uri (nbd);
 
   /* Prefer the server's version of the name, if available */
@@ -116,7 +121,10 @@ show_one_export (struct nbd_handle *nbd, const char *desc,
     fprintf (fp, ":\n");
     if (desc && *desc)
       fprintf (fp, "\tdescription: %s\n", desc);
-    fprintf (fp, "\texport-size: %" PRIi64 "\n", size);
+    if (human_size_flag)
+      fprintf (fp, "\texport-size: %" PRIi64 " (%s)\n",
size, size_str);
+    else
+      fprintf (fp, "\texport-size: %" PRIi64 "\n", size);
     if (content)
       fprintf (fp, "\tcontent: %s\n", content);
     if (uri)
@@ -239,7 +247,8 @@ show_one_export (struct nbd_handle *nbd, const char *desc,
                block_maximum);
 
     /* Put this one at the end because of the stupid comma thing in JSON. */
-    fprintf (fp, "\t\"export-size\": %" PRIi64
"\n", size);
+    fprintf (fp, "\t\"export-size\": %" PRIi64
",\n", size);
+    fprintf (fp, "\t\"export-size-str\":
\"%s\"\n", size_str);
 
     if (last)
       fprintf (fp, "\t} ]\n");
-- 
2.32.0

---
 copy/main.c          | 6 +++++-
 copy/test-verbose.sh | 4 ++--
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/copy/main.c b/copy/main.c
index 70534b5a..15a64544 100644
--- a/copy/main.c
+++ b/copy/main.c
@@ -39,6 +39,7 @@
 #include <libnbd.h>
 
 #include "ispowerof2.h"
+#include "human-size.h"
 #include "version.h"
 #include "nbdcopy.h"
 
@@ -508,8 +509,11 @@ open_local (const char *filename, direction d)
 static void
 print_rw (struct rw *rw, const char *prefix, FILE *fp)
 {
+  char buf[HUMAN_SIZE_LONGEST];
+
   fprintf (fp, "%s: %s \"%s\"\n", prefix,
rw->ops->ops_name, rw->name);
-  fprintf (fp, "%s: size=%" PRIi64 "\n", prefix,
rw->size);
+  fprintf (fp, "%s: size=%" PRIi64 " (%s)\n",
+           prefix, rw->size, human_size (buf, rw->size, NULL));
 }
 
 /* Default implementation of rw->ops->get_extents for backends which
diff --git a/copy/test-verbose.sh b/copy/test-verbose.sh
index afd57580..4cc67d37 100755
--- a/copy/test-verbose.sh
+++ b/copy/test-verbose.sh
@@ -28,11 +28,11 @@ requires nbdkit --version
 file=test-verbose.out
 cleanup_fn rm -f $file
 
-$VG nbdcopy -v -- [ nbdkit null ] null: 2>$file
+$VG nbdcopy -v -- [ nbdkit memory 1M ] null: 2>$file
 
 cat $file
 
 # Check some known strings appear in the output.
 grep '^nbdcopy: src: nbd_ops' $file
-grep '^nbdcopy: src: size=0' $file
+grep '^nbdcopy: src: size=1048576 (1M)' $file
 grep '^nbdcopy: dst: null_ops' $file
-- 
2.32.0

ACK to these three.

On Mon, Sep 20, 2021 at 12:04:46PM +0100, Richard W.M. Jones
wrote:
>For example 1024 is returned as "1K".
>
>This does not attempt to handle decimals or SI units.  If the number
>isn't some multiple of a power of 1024 then it is returned as bytes (a
>flag is available to indicate this).
>
>I looked at both the gnulib and qemu versions of this function.  The
>gnulib version is not under a license which is compatible with libnbd
>and is also really complicated, although it does handle fractions and
>SI units.  The qemu version is essentially just frexp + sprintf and
>doesn't attempt to convert to the human-readable version reversibly.
>---
> .gitignore                     |  1 +
> common/utils/Makefile.am       | 10 +++-
> common/utils/human-size.c      | 54 +++++++++++++++++++++
> common/utils/human-size.h      | 49 +++++++++++++++++++
> common/utils/test-human-size.c | 89 ++++++++++++++++++++++++++++++++++
> 5 files changed, 201 insertions(+), 2 deletions(-)
>
>diff --git a/.gitignore b/.gitignore
>index 2aa1fd99..5fc59677 100644
>--- a/.gitignore
>+++ b/.gitignore
>@@ -31,6 +31,7 @@ Makefile.in
> /bash/nbdcopy
> /bash/nbdfuse
> /bash/nbdinfo
>+/common/utils/test-human-size
> /common/utils/test-vector
> /compile
> /config.cache
>diff --git a/common/utils/Makefile.am b/common/utils/Makefile.am
>index 1ca4a370..b273ada1 100644
>--- a/common/utils/Makefile.am
>+++ b/common/utils/Makefile.am
>@@ -34,6 +34,8 @@ include $(top_srcdir)/common-rules.mk
> noinst_LTLIBRARIES = libutils.la
>
> libutils_la_SOURCES = \
>+	human-size.c \
>+	human-size.h \
> 	vector.c \
> 	vector.h \
> 	version.c \
>@@ -50,8 +52,12 @@ libutils_la_LIBADD = \
>
> # Unit tests.
>
>-TESTS = test-vector
>-check_PROGRAMS = test-vector
>+TESTS = test-human-size test-vector
>+check_PROGRAMS = test-human-size test-vector
>+
>+test_human_size_SOURCES = test-human-size.c human-size.c human-size.h
>+test_human_size_CPPFLAGS = -I$(srcdir)
>+test_human_size_CFLAGS = $(WARNINGS_CFLAGS)
>
> test_vector_SOURCES = test-vector.c vector.c vector.h
> test_vector_CPPFLAGS = -I$(srcdir)
>diff --git a/common/utils/human-size.c b/common/utils/human-size.c
>new file mode 100644
>index 00000000..772f2489
>--- /dev/null
>+++ b/common/utils/human-size.c
>@@ -0,0 +1,54 @@
>+/* nbd client library in userspace
>+ * Copyright (C) 2020-2021 Red Hat Inc.
>+ *
>+ * This library is free software; you can redistribute it and/or
>+ * modify it under the terms of the GNU Lesser General Public
>+ * License as published by the Free Software Foundation; either
>+ * version 2 of the License, or (at your option) any later version.
>+ *
>+ * This library is distributed in the hope that it will be useful,
>+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
>+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>+ * Lesser General Public License for more details.
>+ *
>+ * You should have received a copy of the GNU Lesser General Public
>+ * License along with this library; if not, write to the Free Software
>+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
USA
>+ */
>+
>+#include <config.h>
>+
>+#include <stdio.h>
>+#include <stdlib.h>
>+#include <stdbool.h>
>+#include <stdint.h>
>+#include <inttypes.h>
>+
>+#include "human-size.h"
>+
>+char *
>+human_size (char *buf, uint64_t bytes, bool *human)
>+{
>+  static const char *ext[] = { "E", "P", "T",
"G", "M", "K", "" };
>+  size_t i;
>+
>+  if (buf == NULL) {
>+    buf = malloc (HUMAN_SIZE_LONGEST);
>+    if (buf == NULL)
>+      return NULL;
>+  }
>+
>+  /* Work out which extension to use, if any. */
>+  for (i = 6; i >= 0; --i) {
>+    if (bytes == 0 || (bytes & 1023) != 0)
>+      break;
>+    bytes /= 1024;
>+  }
>+
>+  /* Set to flag to true if we're going to add a human-readable
extension. */
>+  if (human)
>+    *human = ext[i][0] != '\0';
>+
>+  snprintf (buf, HUMAN_SIZE_LONGEST, "%" PRIu64 "%s",
bytes, ext[i]);
>+  return buf;
>+}
>diff --git a/common/utils/human-size.h b/common/utils/human-size.h
>new file mode 100644
>index 00000000..9ee78803
>--- /dev/null
>+++ b/common/utils/human-size.h
>@@ -0,0 +1,49 @@
>+/* nbd client library in userspace
>+ * Copyright (C) 2020-2021 Red Hat Inc.
>+ *
>+ * This library is free software; you can redistribute it and/or
>+ * modify it under the terms of the GNU Lesser General Public
>+ * License as published by the Free Software Foundation; either
>+ * version 2 of the License, or (at your option) any later version.
>+ *
>+ * This library is distributed in the hope that it will be useful,
>+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
>+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>+ * Lesser General Public License for more details.
>+ *
>+ * You should have received a copy of the GNU Lesser General Public
>+ * License along with this library; if not, write to the Free Software
>+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
USA
>+ */
>+
>+#ifndef LIBNBD_HUMAN_SIZE_H
>+#define LIBNBD_HUMAN_SIZE_H
>+
>+#include <stdbool.h>
>+#include <stdint.h>
>+
>+/* If you allocate a buffer of at least this length in bytes and pass
>+ * it as the first parameter to human_size, then it will not overrun.
>+ */
>+#define HUMAN_SIZE_LONGEST 64
>+
>+/* Convert bytes to a human-readable string.
>+ *
>+ * This is roughly the opposite of nbdkit_parse_size.  It will convert
>+ * multiples of powers of 1024 to the appropriate human size with the
>+ * right extension like 'M' or 'G'.  Anything that cannot
be converted
>+ * is returned as bytes.  The *human flag is set to true if the output
>+ * was abbreviated to a human-readable size, or false if it is just
>+ * bytes.
>+ *
>+ * If buf == NULL, a buffer is allocated and returned.  In this case
>+ * the returned buffer must be freed.
>+ *
>+ * buf may also be allocated by the caller, in which case it must be
>+ * at least HUMAN_SIZE_LONGEST bytes.
>+ *
>+ * On error the function returns an error and sets errno.
>+ */
>+extern char *human_size (char *buf, uint64_t bytes, bool *human);
>+
>+#endif /* LIBNBD_HUMAN_SIZE_H */
>diff --git a/common/utils/test-human-size.c b/common/utils/test-human-size.c
>new file mode 100644
>index 00000000..d35a21bf
>--- /dev/null
>+++ b/common/utils/test-human-size.c
>@@ -0,0 +1,89 @@
>+/* nbd client library in userspace
>+ * Copyright (C) 2020-2021 Red Hat Inc.
>+ *
>+ * This library is free software; you can redistribute it and/or
>+ * modify it under the terms of the GNU Lesser General Public
>+ * License as published by the Free Software Foundation; either
>+ * version 2 of the License, or (at your option) any later version.
>+ *
>+ * This library is distributed in the hope that it will be useful,
>+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
>+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>+ * Lesser General Public License for more details.
>+ *
>+ * You should have received a copy of the GNU Lesser General Public
>+ * License along with this library; if not, write to the Free Software
>+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
USA
>+ */
>+
>+#include <config.h>
>+
>+#include <stdio.h>
>+#include <stdlib.h>
>+#include <stdbool.h>
>+#include <stdint.h>
>+#include <inttypes.h>
>+#include <string.h>
>+
>+#include "human-size.h"
>+
>+static unsigned errors = 0;
>+
>+static void
>+test (uint64_t bytes, const char *expected, bool expected_human_flag)
>+{
>+  char actual[HUMAN_SIZE_LONGEST];
>+  bool actual_human_flag;
>+
>+  human_size (actual, bytes, &actual_human_flag);
>+
>+  if (strcmp (actual, expected) == 0 ||
>+      actual_human_flag != expected_human_flag) {
>+    printf ("test-human-size: %" PRIu64 " ->
\"%s\" (%s) OK\n",
>+            bytes, actual, actual_human_flag ? "true" :
"false");
>+    fflush (stdout);
>+  }
>+  else {
>+    fprintf (stderr,
>+             "test-human-size: error: test case %" PRIu64
>+             "expected \"%s\" (%s) "
>+             "but returned \"%s\" (%s)\n",
>+             bytes,
>+             expected, expected_human_flag ? "true" :
"false",
>+             actual, actual_human_flag ? "true" :
"false");
>+    errors++;
>+  }
>+}
>+
>+int
>+main (int argc, char *argv[])
>+{
>+  test (0, "0", false);
>+  test (1, "1", false);
>+  test (512, "512", false);
>+  test (1023, "1023", false);
>+  test (1024, "1K", true);
>+  test (1025, "1025", false);
>+  test (2047, "2047", false);
>+  test (2048, "2K", true);
>+  test (3 * 1024, "3K", true);
>+
>+  test (1023 * 1024, "1023K", true);
>+  test (1048575, "1048575", false);
>+  test (1048576, "1M", true);
>+  test (1048577, "1048577", false);
>+
>+  test (UINT64_C(1073741824), "1G", true);
>+
>+  test (UINT64_C(1099511627776), "1T", true);
>+  test (UINT64_C(1099511627777), "1099511627777", false);
>+  test (UINT64_C(1099511627776) + 1024, "1073741825K", true);
>+
>+  test (UINT64_C(1125899906842624), "1P", true);
>+
>+  test ((uint64_t)INT64_MAX+1, "8E", true);
>+  test (UINT64_MAX-1023, "18014398509481983K", true);
>+  test (UINT64_MAX, "18446744073709551615", false);
>+
>+  exit (errors == 0 ? EXIT_SUCCESS : EXIT_FAILURE);
>+}
>-- 
>2.32.0
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL:
<http://listman.redhat.com/archives/libguestfs/attachments/20210920/12557afd/attachment.sig>

On 09/20/21 13:04, Richard W.M. Jones wrote:
> For example 1024 is returned as "1K".
> 
> This does not attempt to handle decimals or SI units.  If the number
> isn't some multiple of a power of 1024 then it is returned as bytes (a
> flag is available to indicate this).
> 
> I looked at both the gnulib and qemu versions of this function.  The
> gnulib version is not under a license which is compatible with libnbd
> and is also really complicated, although it does handle fractions and
> SI units.  The qemu version is essentially just frexp + sprintf and
> doesn't attempt to convert to the human-readable version reversibly.
> ---
>  .gitignore                     |  1 +
>  common/utils/Makefile.am       | 10 +++-
>  common/utils/human-size.c      | 54 +++++++++++++++++++++
>  common/utils/human-size.h      | 49 +++++++++++++++++++
>  common/utils/test-human-size.c | 89 ++++++++++++++++++++++++++++++++++
>  5 files changed, 201 insertions(+), 2 deletions(-)
> 
> diff --git a/.gitignore b/.gitignore
> index 2aa1fd99..5fc59677 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -31,6 +31,7 @@ Makefile.in
>  /bash/nbdcopy
>  /bash/nbdfuse
>  /bash/nbdinfo
> +/common/utils/test-human-size
>  /common/utils/test-vector
>  /compile
>  /config.cache
> diff --git a/common/utils/Makefile.am b/common/utils/Makefile.am
> index 1ca4a370..b273ada1 100644
> --- a/common/utils/Makefile.am
> +++ b/common/utils/Makefile.am
> @@ -34,6 +34,8 @@ include $(top_srcdir)/common-rules.mk
>  noinst_LTLIBRARIES = libutils.la
>  
>  libutils_la_SOURCES = \
> +	human-size.c \
> +	human-size.h \
>  	vector.c \
>  	vector.h \
>  	version.c \
> @@ -50,8 +52,12 @@ libutils_la_LIBADD = \
>  
>  # Unit tests.
>  
> -TESTS = test-vector
> -check_PROGRAMS = test-vector
> +TESTS = test-human-size test-vector
> +check_PROGRAMS = test-human-size test-vector
> +
> +test_human_size_SOURCES = test-human-size.c human-size.c human-size.h
> +test_human_size_CPPFLAGS = -I$(srcdir)
> +test_human_size_CFLAGS = $(WARNINGS_CFLAGS)
>  
>  test_vector_SOURCES = test-vector.c vector.c vector.h
>  test_vector_CPPFLAGS = -I$(srcdir)
> diff --git a/common/utils/human-size.c b/common/utils/human-size.c
> new file mode 100644
> index 00000000..772f2489
> --- /dev/null
> +++ b/common/utils/human-size.c
> @@ -0,0 +1,54 @@
> +/* nbd client library in userspace
> + * Copyright (C) 2020-2021 Red Hat Inc.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
02110-1301 USA
> + */
(1) General question: should we adopt "SPDX-License-Identifier"s? They
are more succinct.
> +
> +#include <config.h>
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <stdbool.h>
> +#include <stdint.h>
> +#include <inttypes.h>
> +
> +#include "human-size.h"
> +
> +char *
> +human_size (char *buf, uint64_t bytes, bool *human)
> +{
> +  static const char *ext[] = { "E", "P",
"T", "G", "M", "K", "" };
(2) The following would be more frugal:

  static const char ext[][2] = { "E", "P", "T",
"G", "M", "K", "" };

(each pointer in the current array is 4 bytes or 8 bytes).
> +  size_t i;
> +
> +  if (buf == NULL) {
> +    buf = malloc (HUMAN_SIZE_LONGEST);
> +    if (buf == NULL)
> +      return NULL;
> +  }
> +
> +  /* Work out which extension to use, if any. */
> +  for (i = 6; i >= 0; --i) {
> +    if (bytes == 0 || (bytes & 1023) != 0)
> +      break;
> +    bytes /= 1024;
> +  }
(3) "size_t" is an unsigned type. When the loop is entered with i==0,
and the "break" is not taken, "i" will flip back to
((size_t)-1), a very
large positive integer. In other words, the controlling expression of
this loop is unfalsifiable.

Functionally, that's not a problem; when we enter the loop with i==0, we
will have shifted out 6*10 = 60 bits on the least significant end, we
only have 4 bits left, and so (bytes == 0 || (bytes & 1023) != 0) will
definitely fire. It's just that the controlling expression of the loop
is misleading (it suggests it has some role, when it doesn't).

(4) Style: I think bit-and (&) should be coupled with bit-shift (>>),
or
else division (/) should be coupled with remainder (%).

(5) It is not necessary to keep the zero check in the loop body. Once we
determine (before the loop) that the input is nonzero, we know it must
have at least one bit that is set. So we can just continue shifting out
runs of ten zero bits, until we hit a run of ten bits that is not
all-bits-zero.

Thus, how about:

  i = 6;
  if (bytes != 0) {
    while ((bytes & 1023) == 0) {
      bytes >>= 10;
      i--;
    }
  }
> +
> +  /* Set to flag to true if we're going to add a human-readable
extension. */
(4) "Set [the] flag to true"?
> +  if (human)
> +    *human = ext[i][0] != '\0';
> +
> +  snprintf (buf, HUMAN_SIZE_LONGEST, "%" PRIu64 "%s",
bytes, ext[i]);
> +  return buf;
> +}
> diff --git a/common/utils/human-size.h b/common/utils/human-size.h
> new file mode 100644
> index 00000000..9ee78803
> --- /dev/null
> +++ b/common/utils/human-size.h
> @@ -0,0 +1,49 @@
> +/* nbd client library in userspace
> + * Copyright (C) 2020-2021 Red Hat Inc.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
02110-1301 USA
> + */
> +
> +#ifndef LIBNBD_HUMAN_SIZE_H
> +#define LIBNBD_HUMAN_SIZE_H
> +
> +#include <stdbool.h>
> +#include <stdint.h>
> +
> +/* If you allocate a buffer of at least this length in bytes and pass
> + * it as the first parameter to human_size, then it will not overrun.
> + */
> +#define HUMAN_SIZE_LONGEST 64
(5) The integer constant expression

  ((sizeof (uint64_t) * 8 + 2) / 3 + 1)

would be more frugal (but we might not care).

If we take the number of three-bit groups in the word, and divide that
by three -- rounded up --, we get the number of necessary octal digits.
The number of decimal digits needed never exceeds the number of octal
digits needed, so this is safe. Add one character for the NUL terminator.)

> +
> +/* Convert bytes to a human-readable string.
> + *
> + * This is roughly the opposite of nbdkit_parse_size.  It will convert
> + * multiples of powers of 1024 to the appropriate human size with the
> + * right extension like 'M' or 'G'.  Anything that cannot
be converted
> + * is returned as bytes.  The *human flag is set to true if the output
> + * was abbreviated to a human-readable size, or false if it is just
> + * bytes.
> + *
> + * If buf == NULL, a buffer is allocated and returned.  In this case
> + * the returned buffer must be freed.
> + *
> + * buf may also be allocated by the caller, in which case it must be
> + * at least HUMAN_SIZE_LONGEST bytes.
> + *
> + * On error the function returns an error and sets errno.
> + */
> +extern char *human_size (char *buf, uint64_t bytes, bool *human);
> +
> +#endif /* LIBNBD_HUMAN_SIZE_H */
> diff --git a/common/utils/test-human-size.c
b/common/utils/test-human-size.c
> new file mode 100644
> index 00000000..d35a21bf
> --- /dev/null
> +++ b/common/utils/test-human-size.c
> @@ -0,0 +1,89 @@
> +/* nbd client library in userspace
> + * Copyright (C) 2020-2021 Red Hat Inc.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
02110-1301 USA
> + */
> +
> +#include <config.h>
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <stdbool.h>
> +#include <stdint.h>
> +#include <inttypes.h>
> +#include <string.h>
> +
> +#include "human-size.h"
> +
> +static unsigned errors = 0;
> +
> +static void
> +test (uint64_t bytes, const char *expected, bool expected_human_flag)
> +{
> +  char actual[HUMAN_SIZE_LONGEST];
> +  bool actual_human_flag;
> +
> +  human_size (actual, bytes, &actual_human_flag);
> +
> +  if (strcmp (actual, expected) == 0 ||
> +      actual_human_flag != expected_human_flag) {
(6) I don't understand this condition. If I understand the branch *body*
correctly, this is the "test succeeded" case. For that, we need:

  strcmp (actual, expected) == 0 &&
  actual_human_flag == expected_human_flag

The negation of this condition (for the "test failed" case) would be:

  strcmp (actual, expected) != 0 ||
  actual_human_flag != expected_human_flag

> +    printf ("test-human-size: %" PRIu64 " ->
\"%s\" (%s) OK\n",
> +            bytes, actual, actual_human_flag ? "true" :
"false");
> +    fflush (stdout);
> +  }
> +  else {
> +    fprintf (stderr,
> +             "test-human-size: error: test case %" PRIu64
> +             "expected \"%s\" (%s) "
(7) Missing space character in the format string between PRIu64 and
"expected".
> +             "but returned \"%s\" (%s)\n",
> +             bytes,
> +             expected, expected_human_flag ? "true" :
"false",
> +             actual, actual_human_flag ? "true" :
"false");
> +    errors++;
> +  }
> +}
> +
> +int
> +main (int argc, char *argv[])
> +{
> +  test (0, "0", false);
> +  test (1, "1", false);
> +  test (512, "512", false);
> +  test (1023, "1023", false);
> +  test (1024, "1K", true);
> +  test (1025, "1025", false);
> +  test (2047, "2047", false);
> +  test (2048, "2K", true);
> +  test (3 * 1024, "3K", true);
> +
> +  test (1023 * 1024, "1023K", true);
> +  test (1048575, "1048575", false);
> +  test (1048576, "1M", true);
> +  test (1048577, "1048577", false);
> +
> +  test (UINT64_C(1073741824), "1G", true);
> +
> +  test (UINT64_C(1099511627776), "1T", true);
> +  test (UINT64_C(1099511627777), "1099511627777", false);
> +  test (UINT64_C(1099511627776) + 1024, "1073741825K", true);
> +
> +  test (UINT64_C(1125899906842624), "1P", true);
> +
> +  test ((uint64_t)INT64_MAX+1, "8E", true);
> +  test (UINT64_MAX-1023, "18014398509481983K", true);
> +  test (UINT64_MAX, "18446744073709551615", false);
> +
> +  exit (errors == 0 ? EXIT_SUCCESS : EXIT_FAILURE);
> +}
> 
Thanks,
Laszlo