[Libguestfs] [PATCH nbdkit 1/2] ocaml: Simplify NBDKit.set_error
Richard W.M. Jones
rjones at redhat.com
Wed Nov 17 17:57:05 UTC 2021
On Mon, Nov 15, 2021 at 02:00:25PM +0100, Laszlo Ersek wrote:
> On 11/15/21 13:14, Richard W.M. Jones wrote:
>
> (aren't you on PTO?)
Eh hem ..
> > -# This is somewhat different from the other tests because we have
> > -# to build an actual plugin here.
>
> (1) I think this comment removal belongs to a separate patch. (Not a
> "hard requirement", just saying.)
Done, with clearer explanation:
https://gitlab.com/nbdkit/nbdkit/-/commit/6b821fdc4ced4fc0af3fe2480d3ca43cb6476a6d
> > + char *args[] = {
> > + "nbdkit", "-s", "--exit-with-parent",
> > + "./test-ocaml-errorcodes-plugin.so", NULL
> > + };
>
> (3) Style (well, at least my personal preference, not sure about nbdkit
> standards): this should be
>
> static const char * const args[] = ...
>
> or at least (cue <https://libguestfs.org/nbd_connect_command.3.html>)
>
> static char *args[] = ...
>
> and in either case it should go to the top of the block (not mixing
> declarations with code).
>
> Feel free to ignore of course if the pattern is already wide-spread in
> nbdkit.
Thie pattern has been copied around over about a dozen tests. How
about attached instead? Compound literals are a fun, little know
feature!
> > + assert (nbd_pread (nbd, buf, 512, 1*512, 0) == -1);
> > + printf ("%s %d\n", nbd_get_error (), nbd_get_errno ());
> > + fflush (stdout);
> > + assert (nbd_get_errno () == EPERM);
> > +
> > + assert (nbd_pread (nbd, buf, 512, 2*512, 0) == -1);
> > + printf ("%s %d\n", nbd_get_error (), nbd_get_errno ());
> > + fflush (stdout);
> > + assert (nbd_get_errno () == EIO);
> > +
> > + assert (nbd_pread (nbd, buf, 512, 3*512, 0) == -1);
> > + printf ("%s %d\n", nbd_get_error (), nbd_get_errno ());
> > + fflush (stdout);
> > + assert (nbd_get_errno () == ENOMEM);
> > +
> > + assert (nbd_pread (nbd, buf, 512, 4*512, 0) == -1);
> > + printf ("%s %d\n", nbd_get_error (), nbd_get_errno ());
> > + fflush (stdout);
> > + assert (nbd_get_errno () == ESHUTDOWN);
> > +
> > + assert (nbd_pread (nbd, buf, 512, 5*512, 0) == -1);
> > + printf ("%s %d\n", nbd_get_error (), nbd_get_errno ());
> > + fflush (stdout);
> > + assert (nbd_get_errno () == EINVAL);
>
> (4) This shouldn't be difficult to turn into a table of { sector, errno
> } pairs, and a loop. Not necessarily for the space savings, but for ease
> of reading. (I've found it hard to discern the only two differences
> between each repetition of the block.)
>
> (5) The fflush(stdout) looks weird; if that really matters (e.g. for
> following the log when it is written to a regular file), then we might
> want to consider setvbuf() instead. Doesn't matter much if you implement
> (4), because in that case, the loop body will contain only one fflush().
I fixed this all up in v2 which I'll post shortly.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW
-------------- next part --------------
>From f0019129681a48e723e9f9ed36ada3ec7183fb2c Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones at redhat.com>
Date: Tue, 16 Nov 2021 10:09:13 +0000
Subject: [PATCH 1/2] tests: Use compound literal array for nbd_connect_command
parameter
Instead of having a separate char *args[] local variable, we can use a
compound literal array (C99 feature). This change is just
refactoring.
---
tests/README.tests | 7 ++++---
tests/test-connect.c | 8 ++++----
tests/test-curl-cookie-script.c | 21 ++++++++++-----------
tests/test-curl-header-script.c | 21 ++++++++++-----------
tests/test-delay.c | 13 ++++++-------
tests/test-layers.c | 27 ++++++++++++++-------------
tests/test-newstyle.c | 10 +++++-----
tests/test-null.c | 8 +++++---
tests/test-oldstyle.c | 10 +++++-----
tests/test-pause.c | 11 ++++++-----
tests/test-random.c | 9 +++++----
tests/test-split.c | 12 ++++++------
12 files changed, 80 insertions(+), 77 deletions(-)
diff --git a/tests/README.tests b/tests/README.tests
index 595c3c19..a55e6958 100644
--- a/tests/README.tests
+++ b/tests/README.tests
@@ -65,9 +65,10 @@ To test a plugin using libnbd
Open a libnbd handle, and configure it using:
- char *args[] = { "nbdkit", "-s", "--exit-with-parent",
- "plugin", <plugin args ...>, NULL };
- nbd_connect_command (nbd, args);
+ nbd_connect_command (nbd,
+ (char *[]) {
+ "nbdkit", "-s", "--exit-with-parent",
+ "plugin", <plugin args ...>, NULL });
Perform tests via libnbd functions.
diff --git a/tests/test-connect.c b/tests/test-connect.c
index f6b494ac..13143f46 100644
--- a/tests/test-connect.c
+++ b/tests/test-connect.c
@@ -53,10 +53,10 @@ main (int argc, char *argv[])
exit (EXIT_FAILURE);
}
- char *args[] = {
- "nbdkit", "-s", "--exit-with-parent", "example1", NULL
- };
- if (nbd_connect_command (nbd, args) == -1) {
+ if (nbd_connect_command (nbd,
+ (char *[]) {
+ "nbdkit", "-s", "--exit-with-parent",
+ "example1", NULL }) == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
}
diff --git a/tests/test-curl-cookie-script.c b/tests/test-curl-cookie-script.c
index 481207b5..45e3d136 100644
--- a/tests/test-curl-cookie-script.c
+++ b/tests/test-curl-cookie-script.c
@@ -104,17 +104,16 @@ main (int argc, char *argv[])
perror ("asprintf");
exit (EXIT_FAILURE);
}
- char *args[] = {
- "nbdkit", "-s", "--exit-with-parent", "-v",
- "curl",
- "-D", "curl.verbose=1",
- "http://localhost/disk",
- "cookie-script=" SCRIPT,
- "cookie-script-renew=1",
- usp_param, /* unix-socket-path=... */
- NULL
- };
- if (nbd_connect_command (nbd, args) == -1) {
+ if (nbd_connect_command (nbd,
+ (char *[]) {
+ "nbdkit", "-s", "--exit-with-parent", "-v",
+ "curl",
+ "-D", "curl.verbose=1",
+ "http://localhost/disk",
+ "cookie-script=" SCRIPT,
+ "cookie-script-renew=1",
+ usp_param, /* unix-socket-path=... */
+ NULL }) == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
}
diff --git a/tests/test-curl-header-script.c b/tests/test-curl-header-script.c
index a151af05..afe16591 100644
--- a/tests/test-curl-header-script.c
+++ b/tests/test-curl-header-script.c
@@ -126,17 +126,16 @@ main (int argc, char *argv[])
perror ("asprintf");
exit (EXIT_FAILURE);
}
- char *args[] = {
- "nbdkit", "-s", "--exit-with-parent", "-v",
- "curl",
- "-D", "curl.verbose=1",
- "http://localhost/disk",
- "header-script=" SCRIPT,
- "header-script-renew=1",
- usp_param, /* unix-socket-path=... */
- NULL
- };
- if (nbd_connect_command (nbd, args) == -1) {
+ if (nbd_connect_command (nbd,
+ (char *[]) {
+ "nbdkit", "-s", "--exit-with-parent", "-v",
+ "curl",
+ "-D", "curl.verbose=1",
+ "http://localhost/disk",
+ "header-script=" SCRIPT,
+ "header-script-renew=1",
+ usp_param, /* unix-socket-path=... */
+ NULL }) == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
}
diff --git a/tests/test-delay.c b/tests/test-delay.c
index 736de529..b0e6f8de 100644
--- a/tests/test-delay.c
+++ b/tests/test-delay.c
@@ -56,13 +56,12 @@ main (int argc, char *argv[])
exit (EXIT_FAILURE);
}
- char *args[] = {
- "nbdkit", "-s", "--exit-with-parent",
- "--filter", "delay",
- "memory", "1M",
- "wdelay=10", NULL
- };
- if (nbd_connect_command (nbd, args) == -1) {
+ if (nbd_connect_command (nbd,
+ (char *[]) {
+ "nbdkit", "-s", "--exit-with-parent",
+ "--filter", "delay",
+ "memory", "1M",
+ "wdelay=10", NULL }) == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
}
diff --git a/tests/test-layers.c b/tests/test-layers.c
index 13c82289..0083b733 100644
--- a/tests/test-layers.c
+++ b/tests/test-layers.c
@@ -157,19 +157,20 @@ main (int argc, char *argv[])
}
/* Start nbdkit. */
- char *args[] = {
- "nbdkit", "--exit-with-parent", "-fvns",
- /* Because of asynchronous shutdown with threads, finalize
- * isn't reliably called unless we disable parallel.
- */
- "-t", "1",
- "--filter", ".libs/test-layers-filter3." SOEXT,
- "--filter", ".libs/test-layers-filter2." SOEXT,
- "--filter", ".libs/test-layers-filter1." SOEXT,
- ".libs/test-layers-plugin." SOEXT,
- "foo=bar",
- NULL};
- if (nbd_connect_command (nbd, args) == -1) {
+ if (nbd_connect_command (nbd,
+ (char *[]) {
+ "nbdkit", "--exit-with-parent", "-fvns",
+ /* Because of asynchronous shutdown with
+ * threads, finalize isn't reliably
+ * called unless we disable parallel.
+ */
+ "-t", "1",
+ "--filter", ".libs/test-layers-filter3." SOEXT,
+ "--filter", ".libs/test-layers-filter2." SOEXT,
+ "--filter", ".libs/test-layers-filter1." SOEXT,
+ ".libs/test-layers-plugin." SOEXT,
+ "foo=bar",
+ NULL }) == -1) {
dprintf (orig_stderr, "nbd_connect_command: %s\n", nbd_get_error ());
exit (EXIT_FAILURE);
}
diff --git a/tests/test-newstyle.c b/tests/test-newstyle.c
index d96c7e44..1f5d1ca3 100644
--- a/tests/test-newstyle.c
+++ b/tests/test-newstyle.c
@@ -49,11 +49,11 @@ main (int argc, char *argv[])
exit (EXIT_FAILURE);
}
- char *args[] = {
- "nbdkit", "-s", "--exit-with-parent",
- "--newstyle", "file", "file-data", NULL
- };
- if (nbd_connect_command (nbd, args) == -1) {
+ if (nbd_connect_command (nbd,
+ (char *[]) {
+ "nbdkit", "-s", "--exit-with-parent",
+ "--newstyle", "file", "file-data",
+ NULL }) == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
}
diff --git a/tests/test-null.c b/tests/test-null.c
index 2205934f..d220999a 100644
--- a/tests/test-null.c
+++ b/tests/test-null.c
@@ -52,9 +52,11 @@ main (int argc, char *argv[])
exit (EXIT_FAILURE);
}
- char *args[] =
- { "nbdkit", "-s", "--exit-with-parent", "null", "100M", NULL };
- if (nbd_connect_command (nbd, args) == -1) {
+ if (nbd_connect_command (nbd,
+ (char *[]) {
+ "nbdkit", "-s", "--exit-with-parent",
+ "null", "100M",
+ NULL }) == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
}
diff --git a/tests/test-oldstyle.c b/tests/test-oldstyle.c
index 5147768a..0afe868f 100644
--- a/tests/test-oldstyle.c
+++ b/tests/test-oldstyle.c
@@ -49,11 +49,11 @@ main (int argc, char *argv[])
exit (EXIT_FAILURE);
}
- char *args[] = {
- "nbdkit", "-s", "--exit-with-parent",
- "--oldstyle", "file", "file-data", NULL
- };
- if (nbd_connect_command (nbd, args) == -1) {
+ if (nbd_connect_command (nbd,
+ (char *[]) {
+ "nbdkit", "-s", "--exit-with-parent",
+ "--oldstyle", "file", "file-data",
+ NULL }) == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
}
diff --git a/tests/test-pause.c b/tests/test-pause.c
index 3c8ee520..e5a970d6 100644
--- a/tests/test-pause.c
+++ b/tests/test-pause.c
@@ -78,11 +78,12 @@ main (int argc, char *argv[])
exit (EXIT_FAILURE);
}
- char *args[] = {
- "nbdkit", "-s", "--exit-with-parent", "--filter", "pause",
- "example1", "pause-control=" SOCKET, NULL
- };
- if (nbd_connect_command (nbd, args) == -1) {
+ if (nbd_connect_command (nbd,
+ (char *[]) {
+ "nbdkit", "-s", "--exit-with-parent",
+ "--filter", "pause",
+ "example1", "pause-control=" SOCKET,
+ NULL }) == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
}
diff --git a/tests/test-random.c b/tests/test-random.c
index 4316d2b3..b7716efa 100644
--- a/tests/test-random.c
+++ b/tests/test-random.c
@@ -72,10 +72,11 @@ main (int argc, char *argv[])
}
snprintf (sizearg, sizeof sizearg, "%d", SIZE);
- char *args[] = {
- "nbdkit", "-s", "--exit-with-parent", "random", sizearg, NULL
- };
- if (nbd_connect_command (nbd, args) == -1) {
+ if (nbd_connect_command (nbd,
+ (char *[]) {
+ "nbdkit", "-s", "--exit-with-parent",
+ "random", sizearg,
+ NULL }) == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
}
diff --git a/tests/test-split.c b/tests/test-split.c
index be53590d..657e5b5a 100644
--- a/tests/test-split.c
+++ b/tests/test-split.c
@@ -51,12 +51,12 @@ main (int argc, char *argv[])
exit (EXIT_FAILURE);
}
- char *args[] = {
- "nbdkit", "-s", "--exit-with-parent",
- "split", "split1", "split2", "file=split3" /* leave file= to test */,
- NULL
- };
- if (nbd_connect_command (nbd, args) == -1) {
+ if (nbd_connect_command (nbd,
+ (char *[]) {
+ "nbdkit", "-s", "--exit-with-parent",
+ "split", "split1", "split2",
+ "file=split3" /* leave file= to test */,
+ NULL }) == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
}
--
2.32.0
More information about the Libguestfs
mailing list