The argument list for those Python method declarations that can grow too wide is always "PyObject *self, PyObject *args"; simply break that fixed arg list to a new line. Effect demonstrated on the longest line in "python/methods.h": Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516 Signed-off-by: Laszlo Ersek <lersek(a)redhat.com&gt; --- generator/Python.ml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/generator/Python.ml b/generator/Python.ml index d493f17a5d6a..87444c54bdf0 100644 --- a/generator/Python.ml +++ b/generator/Python.ml @@ -70,8 +70,9 @@ let List.iter ( fun name -> - pr "extern PyObject *nbd_internal_py_%s (PyObject *self, PyObject *args);\n" - name; + pr "extern PyObject *nbd_internal_py_%s (\n" name; + pr " PyObject *self, PyObject *args\n"; + pr " );\n" ) ([ "create"; "close"; "display_version"; "alloc_aio_buffer";

Best explained with the diff that results in the generated "ocaml/nbd-c.c" source file: (1) "entries" is an API parameter name (of CBArrayAndLen type), so it can be arbitrarily long; pre-patch, it is embedded three times in the same line. Place each occurrence on a separate line. (2) nbd_internal_ocaml_alloc_int64_from_uint32_array() is a humongous function name even for libnbd standards, and even after change (1), it will share a generated line with one instance of the API parameter name. Rename "int64" to "i64" and "uint32" to "u32" in the function's name, to save some characters. While at it, drop a superfluous (harmless) semicolon, dating back to commit 9564dc52abac ("generator: Add OCaml bindings.", 2019-05-30). Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516 Signed-off-by: Laszlo Ersek <lersek(a)redhat.com&gt; --- generator/OCaml.ml | 9 +++++++-- ocaml/helpers.c | 2 +- ocaml/nbd-c.h | 3 +-- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/generator/OCaml.ml b/generator/OCaml.ml index fc29a6a43281..94ab1d589738 100644 --- a/generator/OCaml.ml +++ b/generator/OCaml.ml @@ -524,8 +524,13 @@ let List.iter ( function | CBArrayAndLen (UInt32 n, count) -> - pr " %sv = nbd_internal_ocaml_alloc_int64_from_uint32_array (%s, %s);\n" - n n count; + pr " %sv = " n; + let fncol = output_column () in + let indent = spaces fncol in + pr "nbd_internal_ocaml_alloc_i64_from_u32_array (\n"; + pr "%s %s,\n" indent n; + pr "%s %s\n" indent count; + pr "%s);\n" indent | CBBytesIn (n, len) -> pr " %sv = caml_alloc_initialized_string (%s, %s);\n" n len n | CBInt n | CBUInt n -> diff --git a/ocaml/helpers.c b/ocaml/helpers.c index 8b4b6693a8f1..3361a6968a23 100644 --- a/ocaml/helpers.c +++ b/ocaml/helpers.c @@ -118,7 +118,7 @@ nbd_internal_ocaml_string_list (value ssv) } value -nbd_internal_ocaml_alloc_int64_from_uint32_array (uint32_t *a, size_t len) +nbd_internal_ocaml_alloc_i64_from_u32_array (uint32_t *a, size_t len) { CAMLparam0 (); CAMLlocal2 (v, rv); diff --git a/ocaml/nbd-c.h b/ocaml/nbd-c.h index 8f46415ab16f..f853c84a7756 100644 --- a/ocaml/nbd-c.h +++ b/ocaml/nbd-c.h @@ -61,8 +61,7 @@ extern void nbd_internal_ocaml_raise_error (void) Noreturn; extern void nbd_internal_ocaml_raise_closed (const char *func) Noreturn; extern const char **nbd_internal_ocaml_string_list (value); -extern value nbd_internal_ocaml_alloc_int64_from_uint32_array (uint32_t *, - size_t); +extern value nbd_internal_ocaml_alloc_i64_from_u32_array (uint32_t *, size_t); extern void nbd_internal_unix_sockaddr_to_sa (value, struct sockaddr_storage *, socklen_t *); extern void nbd_internal_ocaml_exception_in_wrapper (const char *, value);

Trivially factor out the "num_args" value, for making a subsequent patch easier to review. Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516 Signed-off-by: Laszlo Ersek <lersek(a)redhat.com&gt; --- generator/OCaml.ml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/generator/OCaml.ml b/generator/OCaml.ml index c49a1f203b9c..a854afd56171 100644 --- a/generator/OCaml.ml +++ b/generator/OCaml.ml @@ -798,14 +798,15 @@ let pr "}\n"; pr "\n"; - if num_params args optargs > 5 then ( + let num_args = num_params args optargs in + if num_args > 5 then ( pr "/* Byte-code compat function because this method has > 5 parameters.\n"; pr " */\n"; pr "value\n"; pr "nbd_internal_ocaml_nbd_%s_byte (value *argv, int argn)\n" name; pr "{\n"; pr " return nbd_internal_ocaml_nbd_%s (" name; - for i = 0 to num_params args optargs - 1 do + for i = 0 to num_args - 1 do if i > 0 then pr ", "; pr "argv[%d]" i done;

We're going to reuse the part of "pr_wrap" that runs "code" while temporarily redirecting the output channel to a buffer. Extract it as "pr_buf". Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516 Signed-off-by: Laszlo Ersek <lersek(a)redhat.com&gt; --- generator/utils.ml | 22 +++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/generator/utils.ml b/generator/utils.ml index 48bd6dd12ba8..5ae87c1ba786 100644 --- a/generator/utils.ml +++ b/generator/utils.ml @@ -151,6 +151,19 @@ let let spaces n = String.make n ' ' +(* Save the current output channel and replace it with a temporary buffer while + * running ‘code’. Return the buffer. + *) +let pr_buf code = + let old_chan = !chan in + let wrapping_col = !col in + let b = Buffer.create 1024 in + chan := Buffer b; + let exn = try code (); None with exn -> Some exn in + chan := old_chan; + col := wrapping_col; + match exn with None -> b | Some exn -> raise exn + (* Wrap the output at maxcol, breaking up lines when a 'c' character * occurs. For example: * foobar = a, b, c, d, e, f, g @@ -165,15 +178,8 @@ let * temporary buffer while running ‘code’. Then we wrap the * buffer and write it to the restored channel. *) - let old_chan = !chan in + let b = pr_buf code in let wrapping_col = !col in - let b = Buffer.create 1024 in - chan := Buffer b; - let exn = try code (); None with exn -> Some exn in - chan := old_chan; - col := wrapping_col; - (match exn with None -> () | Some exn -> raise exn); - let lines = nsplit "\n" (Buffer.contents b) in match lines with | [] -> ()

Add the "pr_wrap_cstr" function, for wrapping C string literals. Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516 Signed-off-by: Laszlo Ersek <lersek(a)redhat.com&gt; --- generator/utils.mli | 1 + generator/utils.ml | 54 ++++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/generator/utils.mli b/generator/utils.mli index 5bbb188fe934..9d6640c87842 100644 --- a/generator/utils.mli +++ b/generator/utils.mli @@ -53,6 +53,7 @@ val val output_to : string -> (unit -> 'a) -> unit val pr : ('a, unit, string, unit) format4 -> 'a val pr_wrap : ?maxcol:int -> char -> (unit -> 'a) -> unit +val pr_wrap_cstr : ?maxcol:int -> (unit -> 'a) -> unit val output_lineno : unit -> int val output_column : unit -> int diff --git a/generator/utils.ml b/generator/utils.ml index 919572b73557..ff1e8a72f7aa 100644 --- a/generator/utils.ml +++ b/generator/utils.ml @@ -213,6 +213,60 @@ let *) pr "%s" (String.concat "\n" rest) +(* Wrap the C string literal output at ‘maxcol’, breaking up lines when a space + * character occurs. For example: + * foobar = "a b c d e f g h i j k" + * └── pr_wrap_cstr ───┘ + * becomes: + * foobar = "a b c d " + * "e f g h " + * "i j k" + * + * Note that: + * - ‘code’ MUST NOT produce the surrounding quotes, + * - ‘code’ MUST NOT produce multiple lines, + * - ‘code’ MUST do its own quoting, + * - space characters produced by ‘code’ cannot be escaped from wrapping. + *) +let pr_wrap_cstr ?(maxcol = 76) code = + (* Just before entering ‘pr_wrap_cstr’, a leading quote must have been + * produced. + *) + let wrapping_col = !col - 1 in + assert (wrapping_col >= 0); + + let b = pr_buf code in + let lines = nsplit "\n" (Buffer.contents b) in + match lines with + | [] -> () + | line :: [] -> + let fields = nsplit " " line in + let nfields = List.length fields in + let indent = spaces wrapping_col in + List.iteri + (fun i field -> + (* Append a space character to each field except the last. *) + let f = if i < nfields - 1 then field ^ " " else field in + + (* Terminate the string literal, insert a line break, and start a + * properly indented new string literal, before printing the field, if + * (a) the field is not the first one in this string literal, and (b) + * printing the field plus a literal-terminating quote would not fit + * in ‘maxcol’. + * + * Note that this way, the literal-terminating quote will always fit + * in ‘maxcol’, except when the *sole* field in the literal is too + * long. + *) + if !col > wrapping_col + 1 && + !col + (String.length f) + 1 > maxcol then + pr "\"\n%s\"" indent; + + (* Print the field. *) + pr "%s" f + ) fields + | _ -> assert false + let output_lineno () = !lineno let output_column () = !col

On Tue, Apr 25, 2023 at 09:10:55AM +0200, Laszlo Ersek wrote: Looks a little funny to not wrap here; but it's generated code, and the context above makes it obvious why it is easier to wrap in both places (when the debug_direct call above has an obvious reason for wrapping). So far, I have no objections to this series, but I've hit the end of my reviewing day. For 1-11: Reviewed-by: Eric Blake <eblake(a)redhat.com&gt; -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org

In "print_trace_enter", when we iterate over the "args" and "optargs" lists with "List.iter" for printing the *values*, we use the "function" shorthand within the functions that get applied to each arg / optarg element. Expand the "function" shorthand to "fun" + "match", so we can more easily modify the code subsequently. This patch is purely refactoring, it does not change the generated output. It is best viewed with "git show -b". Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516 Signed-off-by: Laszlo Ersek <lersek(a)redhat.com&gt; --- generator/C.ml | 42 +++++++++++--------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/generator/C.ml b/generator/C.ml index 0ccf6970d43c..6d880fb953e0 100644 --- a/generator/C.ml +++ b/generator/C.ml @@ -775,27 +775,31 @@ let ) optargs; pr "\""; List.iter ( - function - | Bool n -> pr ", %s ? \"true\" : \"false\"" n - | BytesOut (n, count) - | BytesPersistOut (n, count) -> pr ", %s" count - | BytesIn (n, count) - | BytesPersistIn (n, count) -> - pr ", %s_printable ? %s_printable : \"\", %s" n n count - | Closure { cbname } -> () - | Enum (n, _) -> pr ", %s" n - | Flags (n, _) -> pr ", %s" n - | Fd n | Int n | Int64 n | SizeT n -> pr ", %s" n - | SockAddrAndLen (_, len) -> pr ", (int) %s" len - | Path n | String n | StringList n -> - pr ", %s_printable ? %s_printable : \"\"" n n - | UInt n | UInt32 n | UInt64 n | UIntPtr n -> pr ", %s" n + fun arg -> + (match arg with + | Bool n -> pr ", %s ? \"true\" : \"false\"" n + | BytesOut (n, count) + | BytesPersistOut (n, count) -> pr ", %s" count + | BytesIn (n, count) + | BytesPersistIn (n, count) -> + pr ", %s_printable ? %s_printable : \"\", %s" n n count + | Closure { cbname } -> () + | Enum (n, _) -> pr ", %s" n + | Flags (n, _) -> pr ", %s" n + | Fd n | Int n | Int64 n | SizeT n -> pr ", %s" n + | SockAddrAndLen (_, len) -> pr ", (int) %s" len + | Path n | String n | StringList n -> + pr ", %s_printable ? %s_printable : \"\"" n n + | UInt n | UInt32 n | UInt64 n | UIntPtr n -> pr ", %s" n + ) ) args; List.iter ( - function - | OClosure { cbname } -> - pr ", CALLBACK_IS_NULL (%s_callback) ? \"<fun>\" : \"NULL\"" cbname - | OFlags (n, _, _) -> pr ", %s" n + fun optarg -> + (match optarg with + | OClosure { cbname } -> + pr ", CALLBACK_IS_NULL (%s_callback) ? \"<fun>\" : \"NULL\"" cbname + | OFlags (n, _, _) -> pr ", %s" n + ) ) optargs; pr ");\n"; List.iter (

The iteration creating the "args" values and the iteration creating the "optargs" values both print commas as prefixes; in other words, whenever any pattern matches, the "pr" that's invoked starts with a ", " substring. For the sake of a subsequent patch, print commas as suffixes, not as prefixes: - Once we're done outputting the format string, calculate the total number of arguments (args + optargs). - If there is at least one argument to print, follow the format string with a comma-space (", "). - Turn both iterations (i.e., over "args" and "optargs") into numbered iterations (i.e., use "List.iteri"). - From each pattern's own "pr" call, strip the comma-space (", ") prefix. - After the "match", see if the (opt)arg just handled was the last one against the *total* number of arguments; if not, print the comma-space as a suffix. This patch is purely refactoring, it does not change the generated output. The command "git show --color-words" is somewhat helpful for viewing the patch. Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516 Signed-off-by: Laszlo Ersek <lersek(a)redhat.com&gt; --- generator/C.ml | 45 ++++++++++++-------- 1 file changed, 27 insertions(+), 18 deletions(-) diff --git a/generator/C.ml b/generator/C.ml index 38b7580b39af..b4118bf41b3a 100644 --- a/generator/C.ml +++ b/generator/C.ml @@ -774,32 +774,41 @@ let | OFlags (n, _, _) -> pr " %s=0x%%x" n ) optargs; pr "\""; - List.iter ( - fun arg -> + let num_args = List.length args + and num_optargs = List.length optargs in + let num_allargs = num_args + num_optargs in + if num_allargs > 0 then + pr ", "; + List.iteri ( + fun i arg -> (match arg with - | Bool n -> pr ", %s ? \"true\" : \"false\"" n + | Bool n -> pr "%s ? \"true\" : \"false\"" n | BytesOut (n, count) - | BytesPersistOut (n, count) -> pr ", %s" count + | BytesPersistOut (n, count) -> pr "%s" count | BytesIn (n, count) | BytesPersistIn (n, count) -> - pr ", %s_printable ? %s_printable : \"\", %s" n n count - | Closure _ -> pr ", \"<fun>\"" - | Enum (n, _) -> pr ", %s" n - | Flags (n, _) -> pr ", %s" n - | Fd n | Int n | Int64 n | SizeT n -> pr ", %s" n - | SockAddrAndLen (_, len) -> pr ", (int) %s" len + pr "%s_printable ? %s_printable : \"\", %s" n n count + | Closure _ -> pr "\"<fun>\"" + | Enum (n, _) -> pr "%s" n + | Flags (n, _) -> pr "%s" n + | Fd n | Int n | Int64 n | SizeT n -> pr "%s" n + | SockAddrAndLen (_, len) -> pr "(int) %s" len | Path n | String n | StringList n -> - pr ", %s_printable ? %s_printable : \"\"" n n - | UInt n | UInt32 n | UInt64 n | UIntPtr n -> pr ", %s" n - ) + pr "%s_printable ? %s_printable : \"\"" n n + | UInt n | UInt32 n | UInt64 n | UIntPtr n -> pr "%s" n + ); + if i < num_allargs - 1 then + pr ", " ) args; - List.iter ( - fun optarg -> + List.iteri ( + fun i optarg -> (match optarg with | OClosure { cbname } -> - pr ", CALLBACK_IS_NULL (%s_callback) ? \"<fun>\" : \"NULL\"" cbname - | OFlags (n, _, _) -> pr ", %s" n - ) + pr "CALLBACK_IS_NULL (%s_callback) ? \"<fun>\" : \"NULL\"" cbname + | OFlags (n, _, _) -> pr "%s" n + ); + if num_args + i < num_allargs - 1 then + pr ", " ) optargs; pr ");\n"; List.iter (

At this point we have ensured that "pr_wrap_cstr" can wrap the "enter" format string, and that "pr_wrap" can wrap the arg/optarg value list. Extract the generation of the format string to "print_format_string" (precisely between the outer quotes), and pass it to "pr_wrap_cstr". If there is at least one argument (arg or optarg alike), don't print a comma-space; after the comma, return the carriage to a new line, and properly indent it, so that it match the name of the debug() vs. debug_direct() function that's being called. This ensures the next "pr_wrap" has maximum width to operate with. This is why we wanted the comma-spaces to be suffixes, rather than prefixes -- we wanted to insert this line break between the first comma after the format string, and the first arg (or optarg, whichever comes first). Extract the generation of the value (arg/optarg) lists to "print_args" (which now only produces inner commas!, see again the prefix->suffix change), and pass "print_args" to "pr_wrap". The longest debug("enter") invocation changes as follows [lib/api.c]: The second longest one [lib/api.c]: There is one debug_direct("enter") invocation change [lib/api.c]: This patch is easiest to view with "git show -b". Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516 Signed-off-by: Laszlo Ersek <lersek(a)redhat.com&gt; --- generator/C.ml | 128 ++++++++++---------- 1 file changed, 67 insertions(+), 61 deletions(-) diff --git a/generator/C.ml b/generator/C.ml index 43601625bcb0..ce6ff69419a5 100644 --- a/generator/C.ml +++ b/generator/C.ml @@ -744,72 +744,78 @@ let spaces 18 ) in pr "%s\"" indent; - pr "enter:"; - List.iter ( - function - | Bool n -> pr " %s=%%s" n - | BytesOut (n, count) - | BytesPersistOut (n, count) -> pr " %s=<buf> %s=%%zu" n count - | BytesIn (n, count) - | BytesPersistIn (n, count) -> - pr " %s=\\\"%%s\\\" %s=%%zu" n count - | Closure { cbname } -> pr " %s=%%s" cbname - | Enum (n, _) -> pr " %s=%%d" n - | Flags (n, _) -> pr " %s=0x%%x" n - | Fd n | Int n -> pr " %s=%%d" n - | Int64 n -> pr " %s=%%\"PRIi64\"" n - | SizeT n -> pr " %s=%%zu" n - | SockAddrAndLen (n, len) -> pr " %s=<sockaddr> %s=%%d" n len - | Path n - | String n -> pr " %s=%%s" n - | StringList n -> pr " %s=%%s" n - | UInt n -> pr " %s=%%u" n - | UInt32 n -> pr " %s=%%\"PRIu32\"" n - | UInt64 n -> pr " %s=%%\"PRIu64\"" n - | UIntPtr n -> pr " %s=%%\"PRIuPTR\"" n - ) args; - List.iter ( - function - | OClosure { cbname } -> pr " %s=%%s" cbname - | OFlags (n, _, _) -> pr " %s=0x%%x" n - ) optargs; + let print_format_string () = + pr "enter:"; + List.iter ( + function + | Bool n -> pr " %s=%%s" n + | BytesOut (n, count) + | BytesPersistOut (n, count) -> pr " %s=<buf> %s=%%zu" n count + | BytesIn (n, count) + | BytesPersistIn (n, count) -> + pr " %s=\\\"%%s\\\" %s=%%zu" n count + | Closure { cbname } -> pr " %s=%%s" cbname + | Enum (n, _) -> pr " %s=%%d" n + | Flags (n, _) -> pr " %s=0x%%x" n + | Fd n | Int n -> pr " %s=%%d" n + | Int64 n -> pr " %s=%%\"PRIi64\"" n + | SizeT n -> pr " %s=%%zu" n + | SockAddrAndLen (n, len) -> pr " %s=<sockaddr> %s=%%d" n len + | Path n + | String n -> pr " %s=%%s" n + | StringList n -> pr " %s=%%s" n + | UInt n -> pr " %s=%%u" n + | UInt32 n -> pr " %s=%%\"PRIu32\"" n + | UInt64 n -> pr " %s=%%\"PRIu64\"" n + | UIntPtr n -> pr " %s=%%\"PRIuPTR\"" n + ) args; + List.iter ( + function + | OClosure { cbname } -> pr " %s=%%s" cbname + | OFlags (n, _, _) -> pr " %s=0x%%x" n + ) optargs + in + pr_wrap_cstr print_format_string; pr "\""; let num_args = List.length args and num_optargs = List.length optargs in let num_allargs = num_args + num_optargs in if num_allargs > 0 then - pr ", "; - List.iteri ( - fun i arg -> - (match arg with - | Bool n -> pr "%s ? \"true\" : \"false\"" n - | BytesOut (n, count) - | BytesPersistOut (n, count) -> pr "%s" count - | BytesIn (n, count) - | BytesPersistIn (n, count) -> - pr "%s_printable ? %s_printable : \"\", %s" n n count - | Closure _ -> pr "\"<fun>\"" - | Enum (n, _) -> pr "%s" n - | Flags (n, _) -> pr "%s" n - | Fd n | Int n | Int64 n | SizeT n -> pr "%s" n - | SockAddrAndLen (_, len) -> pr "(int) %s" len - | Path n | String n | StringList n -> - pr "%s_printable ? %s_printable : \"\"" n n - | UInt n | UInt32 n | UInt64 n | UIntPtr n -> pr "%s" n - ); - if i < num_allargs - 1 then - pr ", " - ) args; - List.iteri ( - fun i optarg -> - (match optarg with - | OClosure { cbname } -> - pr "CALLBACK_IS_NULL (%s_callback) ? \"<fun>\" : \"NULL\"" cbname - | OFlags (n, _, _) -> pr "%s" n - ); - if num_args + i < num_allargs - 1 then - pr ", " - ) optargs; + pr ",\n%s" indent; + let print_args () = + List.iteri ( + fun i arg -> + (match arg with + | Bool n -> pr "%s ? \"true\" : \"false\"" n + | BytesOut (n, count) + | BytesPersistOut (n, count) -> pr "%s" count + | BytesIn (n, count) + | BytesPersistIn (n, count) -> + pr "%s_printable ? %s_printable : \"\", %s" n n count + | Closure _ -> pr "\"<fun>\"" + | Enum (n, _) -> pr "%s" n + | Flags (n, _) -> pr "%s" n + | Fd n | Int n | Int64 n | SizeT n -> pr "%s" n + | SockAddrAndLen (_, len) -> pr "(int) %s" len + | Path n | String n | StringList n -> + pr "%s_printable ? %s_printable : \"\"" n n + | UInt n | UInt32 n | UInt64 n | UIntPtr n -> pr "%s" n + ); + if i < num_allargs - 1 then + pr ", " + ) args; + List.iteri ( + fun i optarg -> + (match optarg with + | OClosure { cbname } -> + pr "CALLBACK_IS_NULL (%s_callback) ? \"<fun>\" : \"NULL\"" cbname + | OFlags (n, _, _) -> pr "%s" n + ); + if num_args + i < num_allargs - 1 then + pr ", " + ) optargs + in + pr_wrap ',' print_args; pr ");\n"; List.iter ( function

The "permitted_state_text" helper function can already fold its output in a certain way, but that's only right for the "generate_docs_nbd_pod" function; i.e., for the "HANDLE STATE" sections of the generated API docs. Therefore, preserve both the folding and non-folding modes of "permitted_state_text", but when we call it (in non-folding mode) from "print_wrapper", apply the previously introduced "pr_wrap_cstr" to the result. The effect on the generated output is [lib/api.c]: Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516 Signed-off-by: Laszlo Ersek <lersek(a)redhat.com&gt; --- generator/C.ml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/generator/C.ml b/generator/C.ml index f7a306de39d8..17b22d84d07d 100644 --- a/generator/C.ml +++ b/generator/C.ml @@ -551,7 +551,9 @@ let pr " set_error (nbd_internal_is_state_created (state) ? ENOTCONN : EINVAL,\n"; pr " \"invalid state: %%s: the handle must be %%s\",\n"; pr " nbd_internal_state_short_string (state),\n"; - pr " \"%s\");\n" (permitted_state_text permitted_states); + pr " \""; + pr_wrap_cstr (fun () -> pr "%s" (permitted_state_text permitted_states)); + pr "\");\n"; pr " return false;\n"; pr " }\n"; pr " return true;\n";

Similarly to the "print_trace_enter" patch earlier, extract the arg list printing logic to a named function, and pass that one to "pr_wrap". Here we need much less work than in "print_trace_enter": no format string (or other C string literal) can be longer than a line, so we can just wrap all arguments in one go. The effect of this patch is [lib/api.c]: The command "git show -b" is useful with this patch. Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516 Signed-off-by: Laszlo Ersek <lersek(a)redhat.com&gt; --- generator/C.ml | 31 +++++++++++--------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/generator/C.ml b/generator/C.ml index 7c8fffd89435..cff8f854f1d1 100644 --- a/generator/C.ml +++ b/generator/C.ml @@ -858,20 +858,23 @@ let ) else pr " debug_direct ("; - if may_set_error then - pr "h, \"leave: ret=" - else - pr "h, \"nbd_%s\", \"leave: ret=" name; - (match ret with - | RBool | RErr | RFd | RInt | REnum _ -> pr "%%d\", ret" - | RInt64 | RCookie -> pr "%%\" PRIi64, ret" - | RSizeT -> pr "%%zd\", ret" - | RString -> pr "%%s\", ret_printable ? ret_printable : \"\"" - | RStaticString -> pr "%%s\", ret" - | RUInt | RFlags _ -> pr "%%u\", ret" - | RUIntPtr -> pr "%%\" PRIuPTR, ret" - | RUInt64 -> pr "%%\" PRIu64, ret" - ); + let print_args () = + if may_set_error then + pr "h, \"leave: ret=" + else + pr "h, \"nbd_%s\", \"leave: ret=" name; + (match ret with + | RBool | RErr | RFd | RInt | REnum _ -> pr "%%d\", ret" + | RInt64 | RCookie -> pr "%%\" PRIi64, ret" + | RSizeT -> pr "%%zd\", ret" + | RString -> pr "%%s\", ret_printable ? ret_printable : \"\"" + | RStaticString -> pr "%%s\", ret" + | RUInt | RFlags _ -> pr "%%u\", ret" + | RUIntPtr -> pr "%%\" PRIuPTR, ret" + | RUInt64 -> pr "%%\" PRIu64, ret" + ) + in + pr_wrap ',' print_args; pr ");\n"; if may_set_error then ( (match ret with

Use a local boolean flag for unnesting the *_in_permitted_state() function call. The one place where this change currently matters is [lib/api.c]: Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516 Signed-off-by: Laszlo Ersek <lersek(a)redhat.com&gt; --- generator/C.ml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/generator/C.ml b/generator/C.ml index cf44424517de..d5ef067fb119 100644 --- a/generator/C.ml +++ b/generator/C.ml @@ -599,7 +599,9 @@ let let value = match errcode with | Some value -> value | None -> assert false in - pr " if (unlikely (!%s_in_permitted_state (h))) {\n" name; + pr " bool p;\n"; + pr " p = %s_in_permitted_state (h);\n" name; + pr " if (unlikely (!p)) {\n"; pr " ret = %s;\n" value; pr " goto out;\n"; pr " }\n";

On 4/26/23 22:25, Eric Blake wrote: Yes, will do! Thanks! Laszlo

Do the same as commit 0744f748ed90 ("generator: indent C argument list 2 spaces relative to function designator", 2023-04-21), except this time not for the header files, but for "lib/api.c". The most impactful resultant change is [lib/api.c]: Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516 Signed-off-by: Laszlo Ersek <lersek(a)redhat.com&gt; --- generator/C.ml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/generator/C.ml b/generator/C.ml index d5ef067fb119..4f1962832eb0 100644 --- a/generator/C.ml +++ b/generator/C.ml @@ -566,7 +566,8 @@ let let ret_c_type = type_of_ret ret and errcode = errcode_of_ret ret in pr "%s\n" ret_c_type; pr "nbd_%s " name; - print_arg_list ~wrap:true ~handle:true args optargs; + print_arg_list ~wrap:true ~handle:true ~parens:(ParensNewLineWithIndent 0) + args optargs; pr "\n"; pr "{\n"; pr " %s ret;\n" ret_c_type;