Re: [Libguestfs] [PATCH libnbd 3/5] common: Combine human-size.h headers into one
On Fri, Oct 06, 2023 at 09:18:31AM -0500, Eric Blake wrote:
> On Sun, Sep 03, 2023 at 04:23:23PM +0100, Richard W.M. Jones wrote:
> > Copy the human_size() function from common/utils/ into the new
> > human-size.h header in common/include/. Remove human-size.c and
> > combine the tests into one.
> > ---
> > common/include/human-size.h | 51 ++
>
> This file was created by inheriting nbdkit's weaker BSD licensing...
>
> > common/include/test-human-size.c | 79 +---
> > common/utils/Makefile.am | 10 +---
> > common/utils/human-size.c| 56
> > common/utils/human-size.h| 49 --
>
> ...while this file was originally created with libnbd's LGPLv2+
> licensing. By merging LGPLv2+ code into a file containing only a BSD
> license header, you have created an ambiguity on what license should
> be assumed when using human_size(). Could you explicitly clarify that
> the relaxing of the license was intentional, so that it is safe to
> then merge libnbd's code into nbdkit without dragging in LGPLv2+ code?
Yes, let's use the weaker (BSD) license for this new code. From the
git history I authored both originally.
> > +static inline char *
> > +human_size (char *buf, uint64_t bytes, bool *human)
> > +{
> > + static const char ext[][2] = { "E", "P", "T", "G", "M", "K", "" };
> > + size_t i;
>
> Code motion, so this is pre-existing, but this seems rather lengthy,
> compared to a more compact:
>
> static const char ext[] = "EPTGMK";
>
> > +
> > + if (buf == NULL) {
> > +buf = malloc (HUMAN_SIZE_LONGEST);
> > +if (buf == NULL)
> > + return NULL;
> > + }
> > +
> > + /* Work out which extension to use, if any. */
> > + i = 6;
> > + if (bytes != 0) {
> > +while ((bytes & 1023) == 0) {
> > + bytes >>= 10;
> > + i--;
> > +}
> > + }
> > +
> > + /* Set the flag to true if we're going to add a human-readable
> > extension. */
> > + if (human)
> > +*human = ext[i][0] != '\0';
>
> *human = ext[i] != '\0';
>
> > +
> > + snprintf (buf, HUMAN_SIZE_LONGEST, "%" PRIu64 "%s", bytes, ext[i]);
>
> snprintf (buf, HUMAN_SIZE_LONGEST, "%" PRIu64 ".1s", bytes, [i]);
>
> > + return buf;
> > +}
> > +
Do you want to suggest / push a patch for these bits?
Thanks,
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH libnbd 3/5] common: Combine human-size.h headers into one
On Sun, Sep 03, 2023 at 04:23:23PM +0100, Richard W.M. Jones wrote:
> Copy the human_size() function from common/utils/ into the new
> human-size.h header in common/include/. Remove human-size.c and
> combine the tests into one.
> ---
> common/include/human-size.h | 51 ++
This file was created by inheriting nbdkit's weaker BSD licensing...
> common/include/test-human-size.c | 79 +---
> common/utils/Makefile.am | 10 +---
> common/utils/human-size.c| 56
> common/utils/human-size.h| 49 --
...while this file was originally created with libnbd's LGPLv2+
licensing. By merging LGPLv2+ code into a file containing only a BSD
license header, you have created an ambiguity on what license should
be assumed when using human_size(). Could you explicitly clarify that
the relaxing of the license was intentional, so that it is safe to
then merge libnbd's code into nbdkit without dragging in LGPLv2+ code?
> +static inline char *
> +human_size (char *buf, uint64_t bytes, bool *human)
> +{
> + static const char ext[][2] = { "E", "P", "T", "G", "M", "K", "" };
> + size_t i;
Code motion, so this is pre-existing, but this seems rather lengthy,
compared to a more compact:
static const char ext[] = "EPTGMK";
> +
> + if (buf == NULL) {
> +buf = malloc (HUMAN_SIZE_LONGEST);
> +if (buf == NULL)
> + return NULL;
> + }
> +
> + /* Work out which extension to use, if any. */
> + i = 6;
> + if (bytes != 0) {
> +while ((bytes & 1023) == 0) {
> + bytes >>= 10;
> + i--;
> +}
> + }
> +
> + /* Set the flag to true if we're going to add a human-readable extension.
> */
> + if (human)
> +*human = ext[i][0] != '\0';
*human = ext[i] != '\0';
> +
> + snprintf (buf, HUMAN_SIZE_LONGEST, "%" PRIu64 "%s", bytes, ext[i]);
snprintf (buf, HUMAN_SIZE_LONGEST, "%" PRIu64 ".1s", bytes, [i]);
> + return buf;
> +}
> +
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH libnbd 3/5] common: Combine human-size.h headers into one
On 9/3/23 17:23, Richard W.M. Jones wrote: > Copy the human_size() function from common/utils/ into the new > human-size.h header in common/include/. Remove human-size.c and > combine the tests into one. > --- > common/include/human-size.h | 51 ++ > common/include/test-human-size.c | 79 +--- > common/utils/Makefile.am | 10 +--- > common/utils/human-size.c| 56 > common/utils/human-size.h| 49 -- > common/utils/test-human-size.c | 89 > 6 files changed, 126 insertions(+), 208 deletions(-) Hopefully you won't get too many rebase conflicts here... Reviewed-by: Laszlo Ersek ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [PATCH libnbd 3/5] common: Combine human-size.h headers into one
Copy the human_size() function from common/utils/ into the new
human-size.h header in common/include/. Remove human-size.c and
combine the tests into one.
---
common/include/human-size.h | 51 ++
common/include/test-human-size.c | 79 +---
common/utils/Makefile.am | 10 +---
common/utils/human-size.c| 56
common/utils/human-size.h| 49 --
common/utils/test-human-size.c | 89
6 files changed, 126 insertions(+), 208 deletions(-)
diff --git a/common/include/human-size.h b/common/include/human-size.h
index 788dbd7ba5..47729c3c58 100644
--- a/common/include/human-size.h
+++ b/common/include/human-size.h
@@ -134,4 +134,55 @@ human_size_parse (const char *str,
return size * scale;
}
+/* 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.
+ */
+static inline char *
+human_size (char *buf, uint64_t bytes, bool *human)
+{
+ static const char ext[][2] = { "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. */
+ i = 6;
+ if (bytes != 0) {
+while ((bytes & 1023) == 0) {
+ bytes >>= 10;
+ i--;
+}
+ }
+
+ /* Set the 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;
+}
+
#endif /* NBDKIT_HUMAN_SIZE_H */
diff --git a/common/include/test-human-size.c b/common/include/test-human-size.c
index e8cbe7f413..d56186b9da 100644
--- a/common/include/test-human-size.c
+++ b/common/include/test-human-size.c
@@ -36,15 +36,18 @@
#include
#include
#include
+#include
#include "array-size.h"
#include "human-size.h"
-int
-main (void)
+static unsigned errors = 0;
+
+/* Test the human_size_parse function. */
+static void
+test1 (void)
{
size_t i;
- bool pass = true;
struct pair {
const char *str;
int64_t res;
@@ -119,15 +122,79 @@ main (void)
fprintf (stderr,
"Wrong parse for %s, got %" PRId64 ", expected %" PRId64 "\n",
pairs[i].str, r, pairs[i].res);
- pass = false;
+ errors++;
}
if (r == -1) {
if (error == NULL || pstr == NULL) {
fprintf (stderr, "Wrong error message handling for %s\n",
pairs[i].str);
-pass = false;
+errors++;
}
}
}
+}
+
+/* Test the human_size function. */
+static void
+test2_run (uint64_t bytes, const char *expected, bool expected_human_flag)
+{
+ char actual[HUMAN_SIZE_LONGEST];
+ bool actual_human_flag;
+
+ human_size (actual, bytes, _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++;
+ }
+}
+
+static void
+test2 (void)
+{
+ test2_run (0, "0", false);
+ test2_run (1, "1", false);
+ test2_run (512, "512", false);
+ test2_run (1023, "1023", false);
+ test2_run (1024, "1K", true);
+ test2_run (1025, "1025", false);
+ test2_run (2047, "2047", false);
+ test2_run (2048, "2K", true);
+ test2_run (3 * 1024, "3K", true);
+
+ test2_run (1023 * 1024, "1023K", true);
+ test2_run (1048575, "1048575", false);
+ test2_run (1048576, "1M", true);
+ test2_run (1048577, "1048577", false);
+
+ test2_run (UINT64_C (1073741824), "1G", true);
+
+ test2_run (UINT64_C (1099511627776), "1T", true);
+ test2_run (UINT64_C (109951162), "109951162", false);
+ test2_run (UINT64_C (1099511627776) + 1024, "1073741825K", true);
+
+ test2_run (UINT64_C
