On Sat, Sep 18, 2021 at 03:38:52PM +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..5cff722b
--- /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;
+  }
+
+  /* Set to false if we're not going to add a human-readable extension. */
+  if (human)
+    *human = bytes != 0 && (bytes & 1023) == 0;
+

It would be more readable (with some de-duplication even) to remove this part 
^^ ...

+  /* Work out which extension to use, if any. */
+  for (i = 6; i >= 0; --i) {
+    if (bytes == 0 || (bytes & 1023) != 0)
+      break;
+    bytes /= 1024;
+  }
+

And just set here:

*human = i == 6;

Even better make it more future-proof and use (sizeof(ext) / sizeof(*ext))
instead of just "6".  Disclaimer: Written by hand, not tested.

+  snprintf (buf, HUMAN_SIZE_LONGEST-1, "%" PRIu64 "%s", bytes, ext[i]);

The '-1' seems unnecessary here as snprintf() counts the zero terminator as part
the supplied size and will not overrun @size-sized buffers.

But even without the modifications both patches look fine.

Martin

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Libguestfs mailing list
[email protected]
https://listman.redhat.com/mailman/listinfo/libguestfs

Reply via email to