Libguestfs - Apr 2023 - [libnbd PATCH 17/18] ublk/nbdublk: wrap source code at 80 characters

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516

This series wraps the non-generated C-language source code (*.c and *.h
files) at 80 characters.

"ocaml/helpers.c" remains overlong, but I couldn't find a way to
wrap
it: its single overlong line contains the comment

  /* For how we're getting the exception name, see:
   *
https://github.com/libguestfs/libguestfs/blob/5d94be2583d557cfc7f8a8cfee7988abfa45a3f8/daemon/daemon-c.c#L40
   */

and even if I truncate the blob hash to 12 nibbles, the line remains too
long.

The following files are also too wide:

  include/libnbd.h
  lib/api.c
  lib/states-run.c
  lib/states.c
  lib/states.h
  lib/unlocked.h
  ocaml/nbd-c.c
  python/libnbdmod.c
  python/methods.h

but they are all generated; we'll have to discuss them separately.

Laszlo

Laszlo Ersek (18):
  examples/copy-libev: wrap source code at 80 characters
  examples/glib-main-loop: wrap source code at 80 characters
  examples/strict-structured-reads: wrap source code at 80 characters
  fuse/nbdfuse: wrap source code at 80 characters
  generator/states: wrap source code at 80 characters
  interop/{dirty-bitmap,structured-read}: wrap the source code at 80
    chars
  interop/interop: wrap source code at 80 characters
  lib/internal.h: wrap source code at 80 characters
  lib/opt: wrap source code at 80 characters
  python/handle: wrap source code at 80 characters
  tests/closure-lifetimes: wrap source code at 80 characters
  tests/connect-uri: wrap source code at 80 characters
  tests/meta-base-allocation: wrap source code at 80 characters
  tests/newstyle-limited: wrap source code at 80 characters
  tests/oldstyle: wrap source code at 80 characters
  tests/requires: wrap source code at 80 characters
  ublk/nbdublk: wrap source code at 80 characters
  ublk/tgt: wrap source code at 80 characters

 examples/copy-libev.c              |  8 ++--
 examples/glib-main-loop.c          |  6 ++-
 examples/strict-structured-reads.c | 11 +++--
 fuse/nbdfuse.c                     |  4 +-
 generator/states-magic.c           |  3 +-
 generator/states-newstyle-opt-go.c | 12 +++--
 interop/dirty-bitmap.c             | 12 ++---
 interop/interop.c                  |  3 +-
 interop/structured-read.c          | 15 +++----
 lib/internal.h                     |  7 ++-
 lib/opt.c                          |  8 ++--
 python/handle.c                    |  4 +-
 tests/closure-lifetimes.c          | 47 ++++++++------------
 tests/connect-uri.c                |  3 +-
 tests/meta-base-allocation.c       | 13 +++---
 tests/newstyle-limited.c           |  8 ++--
 tests/oldstyle.c                   |  8 ++--
 tests/requires.c                   | 16 ++++++-
 ublk/nbdublk.c                     |  4 +-
 ublk/tgt.c                         |  6 ++-
 20 files changed, 106 insertions(+), 92 deletions(-)


base-commit: 424420a55bab1a8ec9acffe684e0bbd0dc4ec45d

The invocation instructions for the copy-libev example are a bit too wide.
The obvious wrapping

  ./copy-ev \
    nbd+unix:///?socket=/tmp/src.sock \
    nbd+unix:///?socket=/tmp/dst.sock

is not desirable in a C-language comment, because the C standard places
line splicing before comment squashing (translation phases #2 and #3,
respectively). Therefore use shell variables.

While at it, fix a small typo (from commit 6be6a9029abb, "examples: Add
example for integrating with libev", 2021-03-01): the executable is called
"copy-libev", not "copy-ev".

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 examples/copy-libev.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/examples/copy-libev.c b/examples/copy-libev.c
index 02f9cad492b9..32cb46b3d815 100644
--- a/examples/copy-libev.c
+++ b/examples/copy-libev.c
@@ -7,9 +7,11 @@
  *
  * To run it:
  *
- *     nbdkit -r pattern size=1G -U /tmp/src.sock
- *     nbdkit memory size=1g -U /tmp/dst.sock
- *     ./copy-ev nbd+unix:///?socket=/tmp/src.sock
nbd+unix:///?socket=/tmp/dst.sock
+ *     SRC=/tmp/src.sock
+ *     DST=/tmp/dst.sock
+ *     nbdkit -r pattern size=1G -U $SRC
+ *     nbdkit memory size=1g -U $DST
+ *     ./copy-libev nbd+unix:///?socket=$SRC nbd+unix:///?socket=$DST
  *
  * To debug it:
  *

Insert straight-forward line breaks into some compound literals, for
keeping the source code width <= 80 chars.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 examples/glib-main-loop.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/examples/glib-main-loop.c b/examples/glib-main-loop.c
index 1982f94131cc..8403a9708327 100644
--- a/examples/glib-main-loop.c
+++ b/examples/glib-main-loop.c
@@ -384,7 +384,8 @@ read_data (gpointer user_data)
 
   if (nbd_aio_pread (gssrc->nbd, buffers[i].data,
                      BUFFER_SIZE, buffers[i].offset,
-                     (nbd_completion_callback) { .callback = finished_read,
.user_data = &buffers[i] },
+                     (nbd_completion_callback) { .callback = finished_read,
+                                                 .user_data = &buffers[i]
},
                      0) == -1) {
     fprintf (stderr, "%s\n", nbd_get_error ());
     exit (EXIT_FAILURE);
@@ -431,7 +432,8 @@ write_data (gpointer user_data)
   buffer->state = BUFFER_WRITING;
   if (nbd_aio_pwrite (gsdest->nbd, buffer->data,
                       BUFFER_SIZE, buffer->offset,
-                      (nbd_completion_callback) { .callback = finished_write,
.user_data = buffer },
+                      (nbd_completion_callback) { .callback = finished_write,
+                                                  .user_data = buffer },
                       0) == -1) {
     fprintf (stderr, "%s\n", nbd_get_error ());
     exit (EXIT_FAILURE);

These compound literals are hard to break down to short lines without
making them look ugly (the type names would have to stand alone on a
line); let's use normal local variables instead.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 examples/strict-structured-reads.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/examples/strict-structured-reads.c
b/examples/strict-structured-reads.c
index 1c9664260654..a2596713f3cd 100644
--- a/examples/strict-structured-reads.c
+++ b/examples/strict-structured-reads.c
@@ -222,6 +222,11 @@ main (int argc, char *argv[])
     struct data *d = malloc (sizeof *d);
     struct range *r = malloc (sizeof *r);
     uint64_t offset;
+    nbd_chunk_callback chunk_callback = { .callback = read_chunk,
+                                          .user_data = d };
+    nbd_completion_callback completion_callback = { .callback = read_verify,
+                                                    .user_data = d,
+                                                    .free = free };
 
     assert (d && r);
     offset = rand () % (exportsize - maxsize);
@@ -230,10 +235,8 @@ main (int argc, char *argv[])
     *r = (struct range) { .first = offset, .last = offset + maxsize, };
     *d = (struct data) { .offset = offset, .count = maxsize, .flags = flags,
                          .remaining = r, };
-    if (nbd_aio_pread_structured (nbd, buf, sizeof buf, offset,
-                                  (nbd_chunk_callback) { .callback =
read_chunk, .user_data = d },
-                                  (nbd_completion_callback) { .callback =
read_verify, .user_data = d, .free = free },
-                                  flags) == -1) {
+    if (nbd_aio_pread_structured (nbd, buf, sizeof buf, offset, chunk_callback,
+                                  completion_callback, flags) == -1) {
       fprintf (stderr, "%s\n", nbd_get_error ());
       exit (EXIT_FAILURE);
     }

Beyond breaking up the long string in the error message, replace
"parameter" with the more correct (pedantic...)
"option-argument" term.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 fuse/nbdfuse.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fuse/nbdfuse.c b/fuse/nbdfuse.c
index 8efe2989d5e1..d3eca25632a6 100644
--- a/fuse/nbdfuse.c
+++ b/fuse/nbdfuse.c
@@ -229,8 +229,8 @@ main (int argc, char *argv[])
     case 'C':
       if (sscanf (optarg, "%u", &connections) != 1 ||
           connections < 1 || connections > 1024) {
-        fprintf (stderr, "%s: --connections parameter must be an unsigned
integer >= 1\n",
-                 argv[0]);
+        fprintf (stderr, "%s: --connections option-argument must be an
"
+                 "unsigned integer >= 1\n", argv[0]);
         exit (EXIT_FAILURE);
       }
       break;

Rewrap some error messages to keep the source code width at most 80 chars.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 generator/states-magic.c           |  3 ++-
 generator/states-newstyle-opt-go.c | 12 ++++++++----
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/generator/states-magic.c b/generator/states-magic.c
index b06057b5acd8..7928959ff86c 100644
--- a/generator/states-magic.c
+++ b/generator/states-magic.c
@@ -53,7 +53,8 @@  MAGIC.CHECK_MAGIC:
   }
   else {
     SET_NEXT_STATE (%.DEAD);
-    set_error (0, "handshake: server is not either an oldstyle or fixed
newstyle NBD server");
+    set_error (0, "handshake: server is not either an oldstyle or fixed
"
+               "newstyle NBD server");
     return 0;
   }
   return 0;
diff --git a/generator/states-newstyle-opt-go.c
b/generator/states-newstyle-opt-go.c
index ffd7233985d6..2ef440d43095 100644
--- a/generator/states-newstyle-opt-go.c
+++ b/generator/states-newstyle-opt-go.c
@@ -163,7 +163,8 @@  NEWSTYLE.OPT_GO.CHECK_REPLY:
       case NBD_INFO_EXPORT:
         if (len != sizeof h->sbuf.or.payload.export) {
           SET_NEXT_STATE (%.DEAD);
-          set_error (0, "handshake: incorrect NBD_INFO_EXPORT option reply
length");
+          set_error (0, "handshake: incorrect NBD_INFO_EXPORT option reply
"
+                     "length");
           return 0;
         }
         exportsize = be64toh (h->sbuf.or.payload.export.exportsize);
@@ -176,7 +177,8 @@  NEWSTYLE.OPT_GO.CHECK_REPLY:
       case NBD_INFO_BLOCK_SIZE:
         if (len != sizeof h->sbuf.or.payload.block_size) {
           SET_NEXT_STATE (%.DEAD);
-          set_error (0, "handshake: incorrect NBD_INFO_BLOCK_SIZE option
reply length");
+          set_error (0, "handshake: incorrect NBD_INFO_BLOCK_SIZE option
"
+                     "reply length");
           return 0;
         }
         min = be32toh (h->sbuf.or.payload.block_size.minimum);
@@ -191,7 +193,8 @@  NEWSTYLE.OPT_GO.CHECK_REPLY:
         if (len > sizeof h->sbuf.or.payload.name_desc.info +
NBD_MAX_STRING ||
             len < sizeof h->sbuf.or.payload.name_desc.info) {
           SET_NEXT_STATE (%.DEAD);
-          set_error (0, "handshake: incorrect NBD_INFO_NAME option reply
length");
+          set_error (0, "handshake: incorrect NBD_INFO_NAME option reply
"
+                     "length");
           return 0;
         }
         free (h->canonical_name);
@@ -206,7 +209,8 @@  NEWSTYLE.OPT_GO.CHECK_REPLY:
         if (len > sizeof h->sbuf.or.payload.name_desc.info +
NBD_MAX_STRING ||
             len < sizeof h->sbuf.or.payload.name_desc.info) {
           SET_NEXT_STATE (%.DEAD);
-          set_error (0, "handshake: incorrect NBD_INFO_DESCRIPTION option
reply length");
+          set_error (0, "handshake: incorrect NBD_INFO_DESCRIPTION option
"
+                     "reply length");
           return 0;
         }
         free (h->description);

In "dirty-bitmap.c", the "nbd_extent_callback" compound
literals are not
only too wide, they're also initialized with the same contents. Replace
all three with a common local variable.

The same applies to "structured-read.c", modulo the type being
"nbd_chunk_callback".

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 interop/dirty-bitmap.c    | 12 ++++--------
 interop/structured-read.c | 15 +++++++--------
 2 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/interop/dirty-bitmap.c b/interop/dirty-bitmap.c
index fc32e3352eaf..05f6e9db11ff 100644
--- a/interop/dirty-bitmap.c
+++ b/interop/dirty-bitmap.c
@@ -124,6 +124,7 @@ main (int argc, char *argv[])
   struct nbd_handle *nbd;
   int64_t exportsize;
   struct data data;
+  nbd_extent_callback extent_callback = { .callback = cb, .user_data =
&data };
   char c;
 
   if (argc < 3) {
@@ -153,17 +154,14 @@ main (int argc, char *argv[])
   }
 
   data = (struct data) { .count = 2, };
-  if (nbd_block_status (nbd, exportsize, 0,
-                        (nbd_extent_callback) { .callback = cb, .user_data =
&data },
-                        0) == -1) {
+  if (nbd_block_status (nbd, exportsize, 0, extent_callback, 0) == -1) {
     fprintf (stderr, "%s\n", nbd_get_error ());
     exit (EXIT_FAILURE);
   }
   assert (data.seen_base && data.seen_dirty);
 
   data = (struct data) { .req_one = true, .count = 2, };
-  if (nbd_block_status (nbd, exportsize, 0,
-                        (nbd_extent_callback) { .callback = cb, .user_data =
&data },
+  if (nbd_block_status (nbd, exportsize, 0, extent_callback,
                         LIBNBD_CMD_FLAG_REQ_ONE) == -1) {
     fprintf (stderr, "%s\n", nbd_get_error ());
     exit (EXIT_FAILURE);
@@ -172,9 +170,7 @@ main (int argc, char *argv[])
 
   /* Trigger a failed callback, to prove connection stays up. */
   data = (struct data) { .count = 2, .fail = true, };
-  if (nbd_block_status (nbd, exportsize, 0,
-                        (nbd_extent_callback) { .callback = cb, .user_data =
&data },
-                        0) != -1) {
+  if (nbd_block_status (nbd, exportsize, 0, extent_callback, 0) != -1) {
     fprintf (stderr, "unexpected block status success\n");
     exit (EXIT_FAILURE);
   }
diff --git a/interop/structured-read.c b/interop/structured-read.c
index c2e99c17b174..84eaab0baacc 100644
--- a/interop/structured-read.c
+++ b/interop/structured-read.c
@@ -106,6 +106,8 @@ main (int argc, char *argv[])
   struct nbd_handle *nbd;
   int64_t exportsize;
   struct data data;
+  nbd_chunk_callback chunk_callback = { .callback = read_cb,
+                                        .user_data = &data };
   char c;
 
   if (argc < 2) {
@@ -141,9 +143,8 @@ main (int argc, char *argv[])
 
   memset (rbuf, 2, sizeof rbuf);
   data = (struct data) { .count = 2, };
-  if (nbd_pread_structured (nbd, rbuf, sizeof rbuf, 2048,
-                            (nbd_chunk_callback) { .callback = read_cb,
.user_data = &data },
-                            0) == -1) {
+  if (nbd_pread_structured (nbd, rbuf, sizeof rbuf, 2048, chunk_callback, 0) =+
-1) {
     fprintf (stderr, "%s\n", nbd_get_error ());
     exit (EXIT_FAILURE);
   }
@@ -152,8 +153,7 @@ main (int argc, char *argv[])
   /* Repeat with DF flag. */
   memset (rbuf, 2, sizeof rbuf);
   data = (struct data) { .df = true, .count = 1, };
-  if (nbd_pread_structured (nbd, rbuf, sizeof rbuf, 2048,
-                            (nbd_chunk_callback) { .callback = read_cb,
.user_data = &data },
+  if (nbd_pread_structured (nbd, rbuf, sizeof rbuf, 2048, chunk_callback,
                             LIBNBD_CMD_FLAG_DF) == -1) {
     fprintf (stderr, "%s\n", nbd_get_error ());
     exit (EXIT_FAILURE);
@@ -166,9 +166,8 @@ main (int argc, char *argv[])
    */
   memset (rbuf, 2, sizeof rbuf);
   data = (struct data) { .count = 2, .fail = true, };
-  if (nbd_pread_structured (nbd, rbuf, sizeof rbuf, 2048,
-                            (nbd_chunk_callback) { .callback = read_cb,
.user_data = &data },
-                            0) != -1) {
+  if (nbd_pread_structured (nbd, rbuf, sizeof rbuf, 2048, chunk_callback, 0) !+
-1) {
     fprintf (stderr, "unexpected pread callback success\n");
     exit (EXIT_FAILURE);
   }

Wrap a long error message.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 interop/interop.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/interop/interop.c b/interop/interop.c
index 23212f8be82a..20e101d4ae73 100644
--- a/interop/interop.c
+++ b/interop/interop.c
@@ -191,7 +191,8 @@ main (int argc, char *argv[])
 #if TLS_FALLBACK
     if (nbd_get_tls_negotiated (nbd) != 0) {
       fprintf (stderr,
-               "%s: TLS disabled, but connection didn't fall back to
plaintext\n",
+               "%s: TLS disabled, but connection didn't fall back to
"
+               "plaintext\n",
                argv[0]);
       exit (EXIT_FAILURE);
     }

(The changes in this patch are simple, but likely more controversial than
the rest.)

The following four components don't play nice together:

- Needlessly spelling out "extern" for function declarations in header
  files. (C99 6.2.2p5: "If the declaration of an identifier for a function
  has no storage-class specifier, its linkage is determined exactly as if
  it were declared with the storage-class specifier extern [...]".)

- Long return type names.

- Very long function names, such as "nbd_internal" +
  ("crypto_create_session" or "state_group_parent").

- Not placing function names seen in declarators (as opposed to those seen
  in definitions) at column #0, lest we confuse utilities that scan for
  "tags".

Shorten lines by breaking the function names to new lines as well, but
indent them to column #2.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 lib/internal.h | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/lib/internal.h b/lib/internal.h
index 2de8e4e5e043..b155681d057f 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -407,7 +407,9 @@ extern int nbd_internal_wait_until_connected (struct
nbd_handle *h)
   LIBNBD_ATTRIBUTE_NONNULL (1);
 
 /* crypto.c */
-extern struct socket *nbd_internal_crypto_create_session (struct nbd_handle *,
struct socket *oldsock)
+extern struct socket *
+  nbd_internal_crypto_create_session (struct nbd_handle *,
+                                      struct socket *oldsock)
   LIBNBD_ATTRIBUTE_NONNULL (1, 2);
 extern bool nbd_internal_crypto_is_reading (struct nbd_handle *)
   LIBNBD_ATTRIBUTE_NONNULL (1);
@@ -504,7 +506,8 @@ extern int nbd_internal_run (struct nbd_handle *h, enum
external_event ev)
   LIBNBD_ATTRIBUTE_NONNULL (1);
 extern const char *nbd_internal_state_short_string (enum state state);
 extern enum state_group nbd_internal_state_group (enum state state);
-extern enum state_group nbd_internal_state_group_parent (enum state_group
group);
+extern enum state_group
+  nbd_internal_state_group_parent (enum state_group group);
 extern int nbd_internal_aio_get_direction (enum state state);
 
 #define set_next_state(h,next_state) ((h)->state) = (next_state)

We're down to truncating function parameter names in this one, with the
functions being nbd_unlocked_aio_opt_list_meta_context_queries() and
nbd_unlocked_aio_opt_set_meta_context_queries().

Note that I don't modify the declarations in "lib/unlocked.h":
first, that
file is generated, and I'm skipping generated files for now; second, the
declarations and the definitions already call the last parameter
differently ("completion_callback" vs. "complete").

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 lib/opt.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/opt.c b/lib/opt.c
index ab3abf860c5c..f58d5e198365 100644
--- a/lib/opt.c
+++ b/lib/opt.c
@@ -467,10 +467,10 @@ int
 nbd_unlocked_aio_opt_list_meta_context_queries (struct nbd_handle *h,
                                                 char **queries,
                                                 nbd_context_callback *context,
-                                                nbd_completion_callback
*complete)
+                                                nbd_completion_callback *compl)
 {
   return aio_opt_meta_context_queries (h, NBD_OPT_LIST_META_CONTEXT,
-                                       queries, context, complete);
+                                       queries, context, compl);
 }
 
 /* Issue NBD_OPT_SET_META_CONTEXT without waiting. */
@@ -488,8 +488,8 @@ int
 nbd_unlocked_aio_opt_set_meta_context_queries (struct nbd_handle *h,
                                                char **queries,
                                                nbd_context_callback *context,
-                                               nbd_completion_callback
*complete)
+                                               nbd_completion_callback *compl)
 {
   return aio_opt_meta_context_queries (h, NBD_OPT_SET_META_CONTEXT,
-                                       queries, context, complete);
+                                       queries, context, compl);
 }

Wrap the message of the #error directive, based on the following (all
excerpts from C99):

- 5.1.1.2 Translation phases, p1, phase 2:
> Each instance of a backslash character (\) immediately followed by a
> new-line character is deleted, splicing physical source lines to form
> logical source lines. [...]"
- 5.1.1.2 Translation phases, p1, phase 3:
> The source file is decomposed into preprocessing tokens [...] and
> sequences of white-space characters (including comments). [...] New-line
> characters are retained. Whether each nonempty sequence of white-space
> characters other than new-line is retained or replaced by one space
> character is implementation-defined.
- 6.10.5 Error directive, p1:
> A preprocessing directive of the form
>
>   # error pp-tokens opt new-line
>
> causes the implementation to produce a diagnostic message that includes
> the specified sequence of preprocessing tokens.
This means that the #error directive does not need a string literal
(preprocessing tokens are a lower-level concept), and that we can break
the sequence of pp-tokens to multiple physical lines with backslashes, and
that an implementation may or may not squeeze our multiple space
characters (even before the patch, after the full stop!) into single space
characters.

In practice, when forcing the #error post-patch, gcc prints (not wrapped
here intentionally):
> handle.c:29:2: error: #error These bindings will not work with Python 2.
Recompile using Python 3 or use ./configure --disable-python.
>    29 | #error \
>       |  ^~~~~
IOW, gcc does squeeze the space characters.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 python/handle.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/python/handle.c b/python/handle.c
index 29a612bbe5f5..2b04cdcda89f 100644
--- a/python/handle.c
+++ b/python/handle.c
@@ -26,7 +26,9 @@
 #include <Python.h>
 
 #if PY_MAJOR_VERSION == 2
-#error "These bindings will not work with Python 2.  Recompile using
Python 3 or use ./configure --disable-python."
+#error \
+  These bindings will not work with Python 2.  \
+  Recompile using Python 3 or use ./configure --disable-python.
 #endif
 
 #include <stdio.h>

The debug, chunk, extent, and completion callbacks are all created with
multiplicated compound initializers. Introduce just one local variable for
each callback structure, also bringing the width of the source file under
80 characters.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 tests/closure-lifetimes.c | 47 ++++++++------------
 1 file changed, 18 insertions(+), 29 deletions(-)

diff --git a/tests/closure-lifetimes.c b/tests/closure-lifetimes.c
index caf0ac275d42..9bb4e120847b 100644
--- a/tests/closure-lifetimes.c
+++ b/tests/closure-lifetimes.c
@@ -117,6 +117,14 @@ main (int argc, char *argv[])
   struct nbd_handle *nbd;
   int64_t cookie;
   char buf[512];
+  nbd_debug_callback debug_callback = { .callback = debug_fn,
+                                        .free = debug_fn_free };
+  nbd_chunk_callback chunk_callback = { .callback = read_cb,
+                                        .free = read_cb_free };
+  nbd_extent_callback extent_callback = { .callback = block_status_cb,
+                                          .free = block_status_cb_free };
+  nbd_completion_callback completion_callback = { .callback = completion_cb,
+                                                  .free = completion_cb_free };
 
   /* Check debug functions are freed when a new debug function is
    * registered, and when the handle is closed.
@@ -124,13 +132,10 @@ main (int argc, char *argv[])
   nbd = nbd_create ();
   if (nbd == NULL) NBD_ERROR;
 
-  nbd_set_debug_callback (nbd,
-                          (nbd_debug_callback) { .callback = debug_fn,
-                                                 .free = debug_fn_free });
+  nbd_set_debug_callback (nbd, debug_callback);
   assert (debug_fn_freed == 0);
 
-  nbd_set_debug_callback (nbd, (nbd_debug_callback) { .callback = debug_fn,
-                                                      .free = debug_fn_free });
+  nbd_set_debug_callback (nbd, debug_callback);
   assert (debug_fn_freed == 1);
 
   debug_fn_freed = 0;
@@ -142,12 +147,8 @@ main (int argc, char *argv[])
   if (nbd == NULL) NBD_ERROR;
   if (nbd_connect_command (nbd, nbdkit) == -1) NBD_ERROR;
 
-  cookie = nbd_aio_pread_structured (nbd, buf, sizeof buf, 0,
-                                     (nbd_chunk_callback) { .callback =
read_cb,
-                                                            .free =
read_cb_free },
-                                     (nbd_completion_callback) { .callback =
completion_cb,
-                                                                 .free =
completion_cb_free },
-                                     0);
+  cookie = nbd_aio_pread_structured (nbd, buf, sizeof buf, 0, chunk_callback,
+                                     completion_callback, 0);
   if (cookie == -1) NBD_ERROR;
   assert (read_cb_freed == 0);
   assert (completion_cb_freed == 0);
@@ -172,12 +173,8 @@ main (int argc, char *argv[])
   if (nbd == NULL) NBD_ERROR;
   if (nbd_connect_command (nbd, nbdkit_delay) == -1) NBD_ERROR;
 
-  cookie = nbd_aio_pread_structured (nbd, buf, sizeof buf, 0,
-                                     (nbd_chunk_callback) { .callback =
read_cb,
-                                                            .free =
read_cb_free },
-                                     (nbd_completion_callback) { .callback =
completion_cb,
-                                                                 .free =
completion_cb_free },
-                                     0);
+  cookie = nbd_aio_pread_structured (nbd, buf, sizeof buf, 0, chunk_callback,
+                                     completion_callback, 0);
   if (cookie == -1) NBD_ERROR;
   nbd_kill_subprocess (nbd, 0);
   nbd_close (nbd);
@@ -195,12 +192,8 @@ main (int argc, char *argv[])
   /* Intentionally omit a call to:
    *  nbd_add_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION);
    */
-  cookie = nbd_aio_block_status (nbd, sizeof buf, 0,
-                                 (nbd_extent_callback) { .callback =
block_status_cb,
-                                                         .free =
block_status_cb_free },
-                                 (nbd_completion_callback) { .callback =
completion_cb,
-                                                             .free =
completion_cb_free },
-                                 0);
+  cookie = nbd_aio_block_status (nbd, sizeof buf, 0, extent_callback,
+                                 completion_callback, 0);
   if (cookie != -1) {
     fprintf (stderr, "%s: Expecting block_status failure\n",
argv[0]);
     exit (EXIT_FAILURE);
@@ -213,12 +206,8 @@ main (int argc, char *argv[])
 
   if (nbd_connect_command (nbd, nbdkit) == -1) NBD_ERROR;
 
-  cookie = nbd_aio_block_status (nbd, sizeof buf, 0,
-                                 (nbd_extent_callback) { .callback =
block_status_cb,
-                                                         .free =
block_status_cb_free },
-                                 (nbd_completion_callback) { .callback =
completion_cb,
-                                                             .free =
completion_cb_free },
-                                 0);
+  cookie = nbd_aio_block_status (nbd, sizeof buf, 0, extent_callback,
+                                 completion_callback, 0);
   if (cookie != -1) {
     fprintf (stderr, "%s: Expecting block_status failure\n",
argv[0]);
     exit (EXIT_FAILURE);

Break up a long error message.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 tests/connect-uri.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/connect-uri.c b/tests/connect-uri.c
index 1896e289186b..f0fd18803a61 100644
--- a/tests/connect-uri.c
+++ b/tests/connect-uri.c
@@ -198,7 +198,8 @@ compare_uris (const char *uri1, const char *uri2)
     if (strstr (uri2, q) != NULL)
       r = 0;
     else {
-      fprintf (stderr, "error: compare_uris: query string '%s'
does not appear in returned URI\n", q);
+      fprintf (stderr, "error: compare_uris: query string '%s'
does not appear "
+               "in returned URI\n", q);
       r = 1;
     }
     free (q);

The extent callback struct is created with a triplicate compound
initializer. Introduce just one local variable for the callback struct,
also bringing the width of the source file under 80 characters.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 tests/meta-base-allocation.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/tests/meta-base-allocation.c b/tests/meta-base-allocation.c
index cc49ba41efee..a7b3af0967f8 100644
--- a/tests/meta-base-allocation.c
+++ b/tests/meta-base-allocation.c
@@ -43,6 +43,8 @@ main (int argc, char *argv[])
   struct nbd_handle *nbd;
   char plugin_path[256];
   int id;
+  nbd_extent_callback extent_callback = { .callback = check_extent,
+                                          .user_data = &id };
   int r;
   const char *s;
   char *tmp;
@@ -148,24 +150,19 @@ main (int argc, char *argv[])
 
   /* Read the block status. */
   id = 1;
-  if (nbd_block_status (nbd, 65536, 0,
-                        (nbd_extent_callback) { .callback = check_extent,
.user_data = &id },
-                        0) == -1) {
+  if (nbd_block_status (nbd, 65536, 0, extent_callback, 0) == -1) {
     fprintf (stderr, "%s\n", nbd_get_error ());
     exit (EXIT_FAILURE);
   }
 
   id = 2;
-  if (nbd_block_status (nbd, 1024, 32768-512,
-                        (nbd_extent_callback) { .callback = check_extent,
.user_data = &id },
-                        0) == -1) {
+  if (nbd_block_status (nbd, 1024, 32768-512, extent_callback, 0) == -1) {
     fprintf (stderr, "%s\n", nbd_get_error ());
     exit (EXIT_FAILURE);
   }
 
   id = 3;
-  if (nbd_block_status (nbd, 1024, 32768-512,
-                        (nbd_extent_callback) { .callback = check_extent,
.user_data = &id },
+  if (nbd_block_status (nbd, 1024, 32768-512, extent_callback,
                         LIBNBD_CMD_FLAG_REQ_ONE) == -1) {
     fprintf (stderr, "%s\n", nbd_get_error ());
     exit (EXIT_FAILURE);

The chunk callback struct is created with a duplicate compound
initializer. Introduce just one local variable for the callback struct,
also bringing the width of the source file under 80 characters.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 tests/newstyle-limited.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/newstyle-limited.c b/tests/newstyle-limited.c
index b86e3614a859..eccca2a24145 100644
--- a/tests/newstyle-limited.c
+++ b/tests/newstyle-limited.c
@@ -113,6 +113,8 @@ main (int argc, char *argv[])
                    "can_write=exit 0",
                    NULL };
   int calls = 0;
+  nbd_chunk_callback chunk_callback = { .callback = pread_cb,
+                                        .user_data = &calls };
   const char *s;
 
   progname = argv[0];
@@ -323,8 +325,7 @@ main (int argc, char *argv[])
   /* Test again for callback operation. */
   memset (rbuf, 0, sizeof rbuf);
   if (nbd_pread_structured (nbd, rbuf, sizeof rbuf, 2 * sizeof rbuf,
-                            (nbd_chunk_callback) { .callback = pread_cb,
.user_data = &calls },
-                            0) == -1) {
+                            chunk_callback, 0) == -1) {
     fprintf (stderr, "%s\n", nbd_get_error ());
     exit (EXIT_FAILURE);
   }
@@ -340,8 +341,7 @@ main (int argc, char *argv[])
 
   /* Also test that callback errors are reflected correctly. */
   if (nbd_pread_structured (nbd, rbuf, sizeof rbuf, 2 * sizeof rbuf,
-                            (nbd_chunk_callback) { .callback = pread_cb,
.user_data = &calls },
-                            0) != -1) {
+                            chunk_callback, 0) != -1) {
     fprintf (stderr, "%s: expected failure from callback\n",
argv[0]);
     exit (EXIT_FAILURE);
   }

The chunk callback struct is created with a duplicate compound
initializer. Introduce just one local variable for the callback struct,
also bringing the width of the source file under 80 characters.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 tests/oldstyle.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/oldstyle.c b/tests/oldstyle.c
index 46fd2a64fcb8..e67c428e63e2 100644
--- a/tests/oldstyle.c
+++ b/tests/oldstyle.c
@@ -84,6 +84,8 @@ main (int argc, char *argv[])
   char *args[] = { "nbdkit", "-s", "-o",
"--exit-with-parent", "-v",
                    "memory", "size=" STR (SIZE), NULL };
   int calls = 0;
+  nbd_chunk_callback chunk_callback = { .callback = pread_cb,
+                                        .user_data = &calls };
   const char *s;
 
   progname = argv[0];
@@ -174,8 +176,7 @@ main (int argc, char *argv[])
   /* Test again for callback operation. */
   memset (rbuf, 0, sizeof rbuf);
   if (nbd_pread_structured (nbd, rbuf, sizeof rbuf, 2 * sizeof rbuf,
-                            (nbd_chunk_callback) { .callback = pread_cb,
.user_data = &calls },
-                            0) == -1) {
+                            chunk_callback, 0) == -1) {
     fprintf (stderr, "%s\n", nbd_get_error ());
     exit (EXIT_FAILURE);
   }
@@ -191,8 +192,7 @@ main (int argc, char *argv[])
 
   /* Also test that callback errors are reflected correctly. */
   if (nbd_pread_structured (nbd, rbuf, sizeof rbuf, 2 * sizeof rbuf,
-                            (nbd_chunk_callback) { .callback = pread_cb,
.user_data = &calls },
-                            0) != -1) {
+                            chunk_callback, 0) != -1) {
     fprintf (stderr, "%s: expected failure from callback\n",
argv[0]);
     exit (EXIT_FAILURE);
   }

Embedding a shell script in a multi-line C string literal is an exercise
in pain. I can see why the original author (whom I shall not look up with
git-blame :) ) went for the easy route. Still, we want the source code to
fit in 80 columns.

Note: in my "interop/test-suite.log", I see
> SKIP: interop-qemu-nbd-tls-certs
> ===============================>
> requires test -d /home/lacos/src/v2v/libnbd/tests/pki
> Test skipped because prerequisite is missing or not working.
> SKIP interop-qemu-nbd-tls-certs (exit status: 77)
>
> SKIP: interop-qemu-nbd-tls-psk
> =============================>
> requires test -f /home/lacos/src/v2v/libnbd/tests/keys.psk
> Test skipped because prerequisite is missing or not working.
> SKIP interop-qemu-nbd-tls-psk (exit status: 77)
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 tests/requires.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/tests/requires.c b/tests/requires.c
index 199e30605473..cc5fd77b9f27 100644
--- a/tests/requires.c
+++ b/tests/requires.c
@@ -57,7 +57,13 @@ requires_qemu_nbd_tls_support (const char *qemu_nbd)
    * interested in the error message that it prints.
    */
   snprintf (cmd, sizeof cmd,
-            "if %s --object tls-creds-x509,id=tls0 |& grep -sq
'TLS credentials support requires GNUTLS'; then exit 1; else exit 0;
fi",
+            "if %s --object tls-creds-x509,id=tls0 \\\n"
+            "    |& grep -sq 'TLS credentials support requires
GNUTLS'\n"
+            "then\n"
+            "    exit 1\n"
+            "else\n"
+            "    exit 0\n"
+            "fi\n",
             qemu_nbd);
   requires (cmd);
 }
@@ -72,7 +78,13 @@ requires_qemu_nbd_tls_psk_support (const char *qemu_nbd)
    * interested in the error message that it prints.
    */
   snprintf (cmd, sizeof cmd,
-            "if %s --object tls-creds-psk,id=tls0 / |& grep -sq
'invalid object type'; then exit 1; else exit 0; fi",
+            "if %s --object tls-creds-psk,id=tls0 / \\\n"
+            "    |& grep -sq 'invalid object type'\n"
+            "then\n"
+            "    exit 1\n"
+            "else\n"
+            "    exit 0\n"
+            "fi\n",
             qemu_nbd);
   requires (cmd);
 }

Beyond breaking up the long string in the error message, replace
"parameter" with the more correct (pedantic...)
"option-argument" term.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 ublk/nbdublk.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ublk/nbdublk.c b/ublk/nbdublk.c
index 3c6ab90037fc..b079d577ae6f 100644
--- a/ublk/nbdublk.c
+++ b/ublk/nbdublk.c
@@ -196,8 +196,8 @@ main (int argc, char *argv[])
     case 'C':
       if (sscanf (optarg, "%u", &connections) != 1 ||
           connections < 1 || connections > 1024) {
-        fprintf (stderr, "%s: --connections parameter must be an unsigned
integer >= 1\n",
-                 argv[0]);
+        fprintf (stderr, "%s: --connections option-argument must be an
"
+                 "unsigned integer >= 1\n", argv[0]);
         exit (EXIT_FAILURE);
       }
       break;

Just break the initializers (for the local constant variables) to new
lines, as seen elsewhere in "ublk/tgt.c" (in the context too, for
example).

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 ublk/tgt.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/ublk/tgt.c b/ublk/tgt.c
index a4a4748b4c03..77d396617fb2 100644
--- a/ublk/tgt.c
+++ b/ublk/tgt.c
@@ -268,7 +268,8 @@ static int
 set_parameters (struct ublksrv_ctrl_dev *ctrl_dev,
                 const struct ublksrv_dev *dev)
 {
-  const struct ublksrv_ctrl_dev_info *dinfo = ublksrv_ctrl_get_dev_info
(ctrl_dev);
+  const struct ublksrv_ctrl_dev_info *dinfo +    ublksrv_ctrl_get_dev_info
(ctrl_dev);
   const unsigned attrs      (readonly ? UBLK_ATTR_READ_ONLY : 0) |
     (rotational ? UBLK_ATTR_ROTATIONAL : 0) |
@@ -308,7 +309,8 @@ set_parameters (struct ublksrv_ctrl_dev *ctrl_dev,
 int
 start_daemon (struct ublksrv_ctrl_dev *ctrl_dev)
 {
-  const struct ublksrv_ctrl_dev_info *dinfo = ublksrv_ctrl_get_dev_info
(ctrl_dev);
+  const struct ublksrv_ctrl_dev_info *dinfo +    ublksrv_ctrl_get_dev_info
(ctrl_dev);
   const struct ublksrv_dev *dev;
   size_t i;
   int r;

On Tue, Apr 18, 2023 at 07:26:13PM +0200, Laszlo Ersek
wrote:
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
[I started this email yesterday, then postponed it while looking at
individual patches...]
> 
> This series wraps the non-generated C-language source code (*.c and *.h
> files) at 80 characters.
> 
> "ocaml/helpers.c" remains overlong, but I couldn't find a way
to wrap
> it: its single overlong line contains the comment
> 
>   /* For how we're getting the exception name, see:
>    *
https://github.com/libguestfs/libguestfs/blob/5d94be2583d557cfc7f8a8cfee7988abfa45a3f8/daemon/daemon-c.c#L40
>    */
> 
> and even if I truncate the blob hash to 12 nibbles, the line remains too
> long.
Truncating is fine to reduce the worst of the width, but I also
understand your reluctance to trim too short (git defaults to 7
nibbles in a fresh repository, but larger repositories like linux.git
output at least 10 nibbles and sometimes more because there are just
that many more hash prefix collisions as history grows - it's never
fun when a link valid today stops working tomorrow when a prefix
collision is introduced into the repo).  At any rate, I have no
problems with long URLs in source files that are otherwise
length-constrained.

> 
> The following files are also too wide:
> 
>   include/libnbd.h
>   lib/api.c
>   lib/states-run.c
>   lib/states.c
>   lib/states.h
>   lib/unlocked.h
>   ocaml/nbd-c.c
>   python/libnbdmod.c
>   python/methods.h
> 
> but they are all generated; we'll have to discuss them separately.
Wrapping a generated file for legibility is definitely harder work;
legible generated code still has its benefits, but longer generated
lines for faster coding of the generator is a tolerable tradeoff in my
book.

Overall, the series looked okay to me at a first read through; I did
spot some things on individual patches where I made comments, but they
are of the nature where I'm also okay with you adding:

Reviewed-by: Eric Blake <eblake at redhat.com>

whether or not you touch things up.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org