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(a)redhat.com&gt; --- 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);

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(a)redhat.com&gt; --- 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;

On Tue, Apr 18, 2023 at 07:26:18PM +0200, Laszlo Ersek wrote: The term 'fixed newstyle' has enough relevance on its own in the protocol that I might wrap one word earlier to keep that phrasing together; but I'm not opposed to your patch going in as-is. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org

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(a)redhat.com&gt; --- 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); }

(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(a)redhat.com&gt; --- 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)

On Wed, Apr 19, 2023 at 11:53:44AM +0100, Richard W.M. Jones wrote: https://stackoverflow.com/questions/18171373/why-are-some-functions-decla... gives some more interesting details (including what happens if you declare a function more than once, using different specifiers across the declarations). I'm okay with dropping extern on functions if you want, but that can be a separate patch; leaving them doesn't hurt either (other than the line length issues where your line splitting techniques are fine). Looks reasonable to me. emacs may have a bit of a difficulty in auto-indenting this back to the same place, but it is infrequent enough that manual override is still viable. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org

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(a)redhat.com&gt; --- 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); }

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(a)redhat.com&gt; --- 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);

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(a)redhat.com&gt; --- 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(a)redhat.com&gt; --- 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); }

On Tue, Apr 18, 2023 at 07:26:29PM +0200, Laszlo Ersek wrote: Note: |& is a bashism; but system() might not invoke bash. So this is already non-portable code; and we should be fixing that. (I don't mind doing it as a followup if you end up pushing before seeing this message). For that matter, 'if ...; then exit 1; else exit 0; fi' can be compressed to '! ...'. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org

On Wed, Apr 19, 2023 at 03:53:57PM +0200, Laszlo Ersek wrote: I don't think there was anything deep here. If '!...' is the same then let's use that. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com nbdkit - Flexible, fast NBD server with plugins https://gitlab.com/nbdkit/nbdkit

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(a)redhat.com&gt; --- 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;