thr3ads.net - Libguestfs - [Libguestfs] [libnbd PATCH 22/23] generator/C: print_wrapper: use helper variable for permitted state check [Apr 2023]

If this information is useful, please help other people find it:
Share via:

Laszlo Ersek

2023-Apr-25 07:10 UTC

[Libguestfs] [libnbd PATCH 00/23] continue wrapping generated C code harder

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
Repo:     https://gitlab.com/lersek/libnbd.git
Branch:   wrap-2172516-wave4

This series is a continuation of the one noted in
<https://bugzilla.redhat.com/show_bug.cgi?id=2172516#c15>. It reduces
the widths of the following generated C files:

  file                width before  width after
  ------------------  ------------  -----------
  include/libnbd.h              81           79
  lib/api.c                    206           79
  ocaml/nbd-c.c                108           79
  python/libnbdmod.c           130           79
  python/methods.h             104           80

The following generated C files remain for the next wave:

  file                width
  ------------------  -----
  lib/states-run.c      102
  lib/states.c          116
  lib/states.h          123

Quite a few patches in the series are easier to review with "git show
-b" or "git show --color-words", due to code reindentation or
other
relatively mechanical changes. (And I've just found out about "git show
--color-moved", which works really well on patch #08, 'generator/utils:
factor out "pr_buf"', for example.) Most patches where such
display
tweaks are beneficial mention that fact.

These formatting tweaks are also the reason why I've pushed the branch
to my libnbd fork on gitlab (see URL & branch name above). The posted
patches should be possible to apply with git-am, for merging (thus, they
couldn't be formatted with the "-b" option, for example), but for
review, it may be easier to just fetch them (or to fetch them in
addition) and to show them locally.

Thanks for reviewing,
Laszlo

Laszlo Ersek (23):
  generator: split LIBNBD_ATTRIBUTE_NONNULL replacement text
  generator: wrap "python/methods.h" at 80 characters
  generator: wrap "python/libnbdmod.c" at 80 characters
  generator: split "int64_from_uint32_array" calls in
"ocaml/nbd-c.c"
  generator: wrap nbd_internal_ocaml_raise_closed() calls in
    "ocaml/nbd-c.c"
  generator: factor out "num_args" in "print_ocaml_binding"
  generator: wrap byte-code compat function bodies in "ocaml/nbd-c.c"
  generator/utils: factor out "pr_buf"
  generator/utils: comment on "maxcol" handling in "pr_wrap"
  generator/utils: add "pr_wrap_cstr"
  generator/C: print_trace_enter: break "enter" to a common new line
  generator/C: print_trace_enter: expand "function" shorthand in value
    iters
  generator/C: print_trace_enter: print Closure values uniformly with
    others
  generator/C: print_trace_enter: generate commas as suffixes, not
    prefixes
  generator/C: print_trace_enter: strip spaces around PRI* macros
  generator/C: print_trace_enter: wrap "enter" debug() /
debug_direct()
    call
  generator/C: print_wrapper: break apart the "ready" &
"processing"
    states
  generator/C: print_wrapper: fold permitted states error message
  generator/C: print_trace_leave: separate arg list from function
    designator
  generator/C: print_trace_leave: wrap "leave" debug() /
debug_direct()
    call
  generator/C: print_flags_check: break guard condition to a new line
  generator/C: print_wrapper: use helper variable for permitted state
    check
  generator/C: lib/api.c: indent arg list 2 spaces relative to function
    name

 generator/C.ml      | 195 ++++++++++++--------
 generator/OCaml.ml  |  27 ++-
 generator/Python.ml |  13 +-
 generator/utils.ml  |  85 ++++++++-
 generator/utils.mli |   1 +
 ocaml/helpers.c     |   2 +-
 ocaml/nbd-c.h       |   3 +-
 7 files changed, 225 insertions(+), 101 deletions(-)


base-commit: 0744f748ed900fb0537da9a5b6532538f3c78e83

Laszlo Ersek

2023-Apr-25 07:10 UTC

head link

[Libguestfs] [libnbd PATCH 01/23] generator: split LIBNBD_ATTRIBUTE_NONNULL replacement text

The LIBNBD_ATTRIBUTE_NONNULL replacement text is just a bit too long;
split the macro definition similarly to LIBNBD_ATTRIBUTE_ALLOC_DEALLOC.

It changes "include/libnbd.h" like this:
>  #ifndef LIBNBD_ATTRIBUTE_NONNULL
>  #if defined (__GNUC__) && LIBNBD_GCC_VERSION >= 120000 /* gcc
>= 12.0 */
> -#define LIBNBD_ATTRIBUTE_NONNULL(...) __attribute__ ((__nonnull__
(__VA_ARGS__)))
> +#define LIBNBD_ATTRIBUTE_NONNULL(...) \
> +  __attribute__ ((__nonnull__ (__VA_ARGS__)))
>  #else
>  #define LIBNBD_ATTRIBUTE_NONNULL(...)
>  #endif
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 generator/C.ml | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/generator/C.ml b/generator/C.ml
index 61ca43d95db5..b65d3de2e82d 100644
--- a/generator/C.ml
+++ b/generator/C.ml
@@ -420,7 +420,8 @@ let
   pr "\n";
   pr "#ifndef LIBNBD_ATTRIBUTE_NONNULL\n";
   pr "#if defined (__GNUC__) && LIBNBD_GCC_VERSION >= 120000 /*
gcc >= 12.0 */\n";
-  pr "#define LIBNBD_ATTRIBUTE_NONNULL(...) __attribute__ ((__nonnull__
(__VA_ARGS__)))\n";
+  pr "#define LIBNBD_ATTRIBUTE_NONNULL(...) \\\n";
+  pr "  __attribute__ ((__nonnull__ (__VA_ARGS__)))\n";
   pr "#else\n";
   pr "#define LIBNBD_ATTRIBUTE_NONNULL(...)\n";
   pr "#endif\n";

Laszlo Ersek

2023-Apr-25 07:10 UTC

head link

[Libguestfs] [libnbd PATCH 02/23] generator: wrap "python/methods.h" at 80 characters

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":
> -extern PyObject *nbd_internal_py_aio_connect_systemd_socket_activation
(PyObject *self, PyObject *args);
> +extern PyObject *nbd_internal_py_aio_connect_systemd_socket_activation (
> +                   PyObject *self, PyObject *args
> +                 );
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 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";

Laszlo Ersek

2023-Apr-25 07:10 UTC

head link

[Libguestfs] [libnbd PATCH 03/23] generator: wrap "python/libnbdmod.c" at 80 characters

Break the fields of each PyMethodDef structure initializer to separate
lines.

While at it, drop the superfluous (char*) cast from the
"PyMethodDef.ml_name" field initializer
<https://docs.python.org/3/c-api/structures.html#c.PyMethodDef>.
(According to commit bf0eee111fc3, "Add Python 3 bindings.",
2019-05-11,
we don't target Python 2.)

Effect shown on the currently longest line in "python/libnbdmod.c":
> -  { (char *) "aio_connect_systemd_socket_activation",
nbd_internal_py_aio_connect_systemd_socket_activation, METH_VARARGS, NULL },
> +  {
> +    "aio_connect_systemd_socket_activation",
> +    nbd_internal_py_aio_connect_systemd_socket_activation,
> +    METH_VARARGS,
> +    NULL
> +  },
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 generator/Python.ml | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/generator/Python.ml b/generator/Python.ml
index 87444c54bdf0..c146f04f0b49 100644
--- a/generator/Python.ml
+++ b/generator/Python.ml
@@ -100,8 +100,12 @@ let
   pr "static PyMethodDef methods[] = {\n";
   List.iter (
     fun name ->
-      pr "  { (char *) \"%s\", nbd_internal_py_%s, METH_VARARGS,
NULL },\n"
-         name name;
+      pr "  {\n";
+      pr "    \"%s\",\n" name;
+      pr "    nbd_internal_py_%s,\n" name;
+      pr "    METH_VARARGS,\n";
+      pr "    NULL\n";
+      pr "  },\n"
   ) ([ "create"; "close";
        "display_version";
        "alloc_aio_buffer";

Laszlo Ersek

2023-Apr-25 07:10 UTC

head link

[Libguestfs] [libnbd PATCH 04/23] generator: split "int64_from_uint32_array" calls in "ocaml/nbd-c.c"

Best explained with the diff that results in the generated
"ocaml/nbd-c.c"
source file:
> --- old 2023-04-21 10:44:30.192602608 +0200
> +++ new 2023-04-21 10:51:46.582658399 +0200
> @@ -205,7 +205,10 @@ extent_wrapper_locked (void *user_data,
>
>    metacontextv = caml_copy_string (metacontext);
>    offsetv = caml_copy_int64 (offset);
> -  entriesv = nbd_internal_ocaml_alloc_int64_from_uint32_array (entries,
nr_entries);
> +  entriesv = nbd_internal_ocaml_alloc_i64_from_u32_array (
> +               entries,
> +               nr_entries
> +             );
>    errorv = caml_alloc_tuple (1);
>    Store_field (errorv, 0, Val_int (*error));
>    args[0] = metacontextv;
(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 at redhat.com>
---
 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);

Laszlo Ersek

2023-Apr-25 07:10 UTC

head link

[Libguestfs] [libnbd PATCH 05/23] generator: wrap nbd_internal_ocaml_raise_closed() calls in "ocaml/nbd-c.c"

Example effect, in
nbd_internal_ocaml_nbd_aio_connect_systemd_socket_activation():
>    struct nbd_handle *h = NBD_val (hv);
>    if (h == NULL)
> -    nbd_internal_ocaml_raise_closed
("NBD.aio_connect_systemd_socket_activation");
> +    nbd_internal_ocaml_raise_closed (
> +      "NBD.aio_connect_systemd_socket_activation"
> +    );
>
>    char **argv = (char **) nbd_internal_ocaml_string_list (argvv);
>    int r;
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 generator/OCaml.ml | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/generator/OCaml.ml b/generator/OCaml.ml
index 94ab1d589738..c49a1f203b9c 100644
--- a/generator/OCaml.ml
+++ b/generator/OCaml.ml
@@ -633,7 +633,9 @@ let
 
   pr "  struct nbd_handle *h = NBD_val (hv);\n";
   pr "  if (h == NULL)\n";
-  pr "    nbd_internal_ocaml_raise_closed (\"NBD.%s\");\n"
name;
+  pr "    nbd_internal_ocaml_raise_closed (\n";
+  pr "      \"NBD.%s\"\n" name;
+  pr "    );\n";
   pr "\n";
 
   List.iter (

Laszlo Ersek

2023-Apr-25 07:10 UTC

head link

[Libguestfs] [libnbd PATCH 06/23] generator: factor out "num_args" in "print_ocaml_binding"

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 at redhat.com>
---
 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;

Laszlo Ersek

2023-Apr-25 07:10 UTC

head link

[Libguestfs] [libnbd PATCH 07/23] generator: wrap byte-code compat function bodies in "ocaml/nbd-c.c"

Wrap the long list of (short) arguments in the byte-code compat function
bodies that we generate for methods with more than 5 parameters.

Effect on "ocaml/nbd-c.c":
> @@ -3920,7 +3920,9 @@
>  value
>  nbd_internal_ocaml_nbd_aio_pread_structured_byte (value *argv, int argn)
>  {
> -  return nbd_internal_ocaml_nbd_aio_pread_structured (argv[0], argv[1],
argv[2], argv[3], argv[4], argv[5]);
> +  return nbd_internal_ocaml_nbd_aio_pread_structured (argv[0], argv[1],
> +                                                      argv[2], argv[3],
> +                                                      argv[4], argv[5]);
>  }
>
>  value
> @@ -4242,7 +4244,8 @@
>  value
>  nbd_internal_ocaml_nbd_aio_block_status_byte (value *argv, int argn)
>  {
> -  return nbd_internal_ocaml_nbd_aio_block_status (argv[0], argv[1],
argv[2], argv[3], argv[4], argv[5]);
> +  return nbd_internal_ocaml_nbd_aio_block_status (argv[0], argv[1],
argv[2],
> +                                                  argv[3], argv[4],
argv[5]);
>  }
>
>  value
The patch is easiest to review with "git show -b".

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

diff --git a/generator/OCaml.ml b/generator/OCaml.ml
index a854afd56171..300c8a70d716 100644
--- a/generator/OCaml.ml
+++ b/generator/OCaml.ml
@@ -806,10 +806,13 @@ let
     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_args - 1 do
-      if i > 0 then pr ", ";
-      pr "argv[%d]" i
-    done;
+    let print_args () +      for i = 0 to num_args - 1 do
+        if i > 0 then pr ", ";
+        pr "argv[%d]" i
+      done
+    in
+    pr_wrap ',' print_args;
     pr ");\n";
     pr "}\n";
     pr "\n"

Laszlo Ersek

2023-Apr-25 07:10 UTC

head link

[Libguestfs] [libnbd PATCH 08/23] generator/utils: factor out "pr_buf"

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 at redhat.com>
---
 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
   | [] -> ()

Laszlo Ersek

2023-Apr-25 07:10 UTC

head link

[Libguestfs] [libnbd PATCH 09/23] generator/utils: comment on "maxcol" handling in "pr_wrap"

Explain in "pr_wrap" why the line break is *only seemingly*
off-by-one; it
is actually correct.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 generator/utils.ml | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/generator/utils.ml b/generator/utils.ml
index 5ae87c1ba786..919572b73557 100644
--- a/generator/utils.ml
+++ b/generator/utils.ml
@@ -186,6 +186,9 @@ let
   | line :: rest ->
      let fields = nsplit (String.make 1 c) line in
      let maybe_wrap field +       (* Note that here we break even if we'd
fill ?maxcol? precisely;
+        * that's because...
+        *)
        if !col > wrapping_col && !col + String.length field >=
maxcol then (
          pr "\n%s" (spaces wrapping_col);
          match span field " \t" with
@@ -197,7 +200,11 @@ let
      let rec loop = function
        | [] -> ()
        | f :: [] -> let f = maybe_wrap f in pr "%s" f;
-       | f :: fs -> let f = maybe_wrap f in pr "%s%c" f c; loop fs
+       | f :: fs ->
+           let f = maybe_wrap f in
+           (* ... here we append the separator. *)
+           pr "%s%c" f c;
+           loop fs
      in
      loop fields;

Laszlo Ersek

2023-Apr-25 07:10 UTC

head link

[Libguestfs] [libnbd PATCH 10/23] generator/utils: add "pr_wrap_cstr"

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 at redhat.com>
---
 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

Laszlo Ersek

2023-Apr-25 07:10 UTC

head link

[Libguestfs] [libnbd PATCH 11/23] generator/C: print_trace_enter: break "enter" to a common new line

Going forward, we'd like to wrap long string literals. The recently
introduced "pr_wrap_cstr" function may only be called right after the
opening quote of the string literal has been produced. In the
"print_trace_enter" function, we start a long string literal with
"enter:"; however, because we either pass the literal to debug() or
debug_direct(), this "enter:" is spelled out twice. Factor
"enter:" out to
a common line, also ensuring that we'll have as much room for wrapping as
possible.

Example effect [lib/api.c]:
> @@ -6220,7 +6360,8 @@ nbd_supports_uri (struct nbd_handle *h)
>
>    /* This function must not call set_error. */
>    if_debug (h) {
> -    debug_direct (h, "nbd_supports_uri", "enter:");
> +    debug_direct (h, "nbd_supports_uri",
> +                  "enter:");
>    }
>
>    ret = nbd_unlocked_supports_uri (h);
> @@ -6260,7 +6401,8 @@ nbd_get_uri (struct nbd_handle *h)
>
>    pthread_mutex_lock (&h->lock);
>    if_debug (h) {
> -    debug (h, "enter:");
> +    debug (h,
> +           "enter:");
>    }
>
>    if (unlikely (!get_uri_in_permitted_state (h))) {
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 generator/C.ml | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/generator/C.ml b/generator/C.ml
index b65d3de2e82d..0ccf6970d43c 100644
--- a/generator/C.ml
+++ b/generator/C.ml
@@ -734,11 +734,17 @@ let
       | Int64 _ | SizeT _
       | SockAddrAndLen _ | UInt _ | UInt32 _ | UInt64 _ | UIntPtr _ -> ()
     ) args;
-    if may_set_error then
-      pr "    debug (h, \"enter:"
-    else (
-      pr "    debug_direct (h, \"nbd_%s\", \"enter:"
name
-    );
+    let indent +      if may_set_error then (
+        pr "    debug (h,\n";
+        spaces 11
+      )
+      else (
+        pr "    debug_direct (h, \"nbd_%s\",\n" name;
+        spaces 18
+      ) in
+    pr "%s\"" indent;
+    pr "enter:";
     List.iter (
       function
       | Bool n -> pr " %s=%%s" n

Laszlo Ersek

2023-Apr-25 07:10 UTC

head link

[Libguestfs] [libnbd PATCH 12/23] generator/C: print_trace_enter: expand "function" shorthand in value iters

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 at redhat.com>
---
 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 (

Laszlo Ersek

2023-Apr-25 07:10 UTC

head link

[Libguestfs] [libnbd PATCH 13/23] generator/C: print_trace_enter: print Closure values uniformly with others

In "print_trace_enter", for a

  Closure { "cbname" }

arg, we create the format string

  cbname=<fun>

and (accordingly) we don't create a value to print.

This is a problem for a subsequent patch. When we iterate over the values
to format, it's only the Closure pattern that does not invoke
"pr"; all
other patterns result in a "pr" call.

In order to handle a Closure arg uniformly with the other types, reuse the
logic seen with the OClosure optarg pattern. Namely, create the format
string

  cbname=%s

and create the value

  "<fun>".

Two examples [lib/api.c]:
> @@ -1925,7 +1925,7 @@ nbd_opt_list_meta_context (struct nbd_ha
>    pthread_mutex_lock (&h->lock);
>    if_debug (h) {
>      debug (h,
> -           "enter: context=<fun>");
> +           "enter: context=%s", "<fun>");
>    }
>
>    if (unlikely (!opt_list_meta_context_in_permitted_state (h))) {
> @@ -1983,7 +1983,7 @@ nbd_opt_list_meta_context_queries (struc
>      char *queries_printable >         
nbd_internal_printable_string_list (queries);
>      debug (h,
> -           "enter: queries=%s context=<fun>",
queries_printable ? queries_printable : "");
> +           "enter: queries=%s context=%s", queries_printable ?
queries_printable : "", "<fun>");
>      free (queries_printable);
>    }
>
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 generator/C.ml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/generator/C.ml b/generator/C.ml
index 6d880fb953e0..38b7580b39af 100644
--- a/generator/C.ml
+++ b/generator/C.ml
@@ -753,7 +753,7 @@ let
       | BytesIn (n, count)
       | BytesPersistIn (n, count) ->
          pr " %s=\\\"%%s\\\" %s=%%zu" n count
-      | Closure { cbname } -> pr " %s=<fun>" cbname
+      | 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
@@ -783,7 +783,7 @@ let
          | BytesIn (n, count)
          | BytesPersistIn (n, count) ->
             pr ", %s_printable ? %s_printable : \"\", %s" n
n count
-         | Closure { cbname } -> ()
+         | Closure _ -> pr ", \"<fun>\""
          | Enum (n, _) -> pr ", %s" n
          | Flags (n, _) -> pr ", %s" n
          | Fd n | Int n | Int64 n | SizeT n -> pr ", %s" n

Laszlo Ersek

2023-Apr-25 07:10 UTC

head link

[Libguestfs] [libnbd PATCH 14/23] generator/C: print_trace_enter: generate commas as suffixes, not prefixes

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 at redhat.com>
---
 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 (

Laszlo Ersek

2023-Apr-25 07:10 UTC

head link

[Libguestfs] [libnbd PATCH 15/23] generator/C: print_trace_enter: strip spaces around PRI* macros

We're going to want to call "pr_wrap_cstr" in a subsequent patch,
for
wrapping format strings. "pr_wrap_cstr" only handles a single C string
literal, but we may generate a format string that actually consists of
multiple string literals. We "exit" and "reenter" string
literals only for
the various PRI* macros. We can get away with this if we remove the space
characters surrounding the PRI* macros; that way, "pr_wrap_cstr" will
not
be tempted to wrap there -- that is, wrap at a space *outside* of a string
literal.

Some examples of the effect this patch has [lib/api.c]:
> @@ -332,7 +332,7 @@ nbd_set_private_data (struct nbd_handle
>    /* This function must not call set_error. */
>    if_debug (h) {
>      debug_direct (h, "nbd_set_private_data",
> -                  "enter: private_data=%" PRIuPTR "",
private_data);
> +                  "enter: private_data=%"PRIuPTR"",
private_data);
>    }
>
>    ret = nbd_unlocked_set_private_data (h, private_data);
> @@ -2613,7 +2613,7 @@ nbd_connect_vsock (struct nbd_handle *h,
>    pthread_mutex_lock (&h->lock);
>    if_debug (h) {
>      debug (h,
> -           "enter: cid=%" PRIu32 " port=%" PRIu32
"", cid, port);
> +           "enter: cid=%"PRIu32"
port=%"PRIu32"", cid, port);
>    }
>
>    if (unlikely (!connect_vsock_in_permitted_state (h))) {
> @@ -3716,7 +3716,7 @@ nbd_pread (struct nbd_handle *h, void *b
>    pthread_mutex_lock (&h->lock);
>    if_debug (h) {
>      debug (h,
> -           "enter: buf=<buf> count=%zu offset=%" PRIu64
" flags=0x%x", count, offset, flags);
> +           "enter: buf=<buf> count=%zu
offset=%"PRIu64" flags=0x%x", count, offset, flags);
>    }
>
>    if (h->pread_initialize)
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 generator/C.ml | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/generator/C.ml b/generator/C.ml
index b4118bf41b3a..43601625bcb0 100644
--- a/generator/C.ml
+++ b/generator/C.ml
@@ -757,16 +757,16 @@ let
       | 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
+      | 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
+      | UInt32 n -> pr " %s=%%\"PRIu32\"" n
+      | UInt64 n -> pr " %s=%%\"PRIu64\"" n
+      | UIntPtr n -> pr " %s=%%\"PRIuPTR\"" n
     ) args;
     List.iter (
       function

Laszlo Ersek

2023-Apr-25 07:11 UTC

head link

[Libguestfs] [libnbd PATCH 16/23] generator/C: print_trace_enter: wrap "enter" debug() / debug_direct() call

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]:
> @@ -5469,7 +5548,10 @@ nbd_aio_pwrite (struct nbd_handle *h, co
>      char *buf_printable >          nbd_internal_printable_buffer (buf,
count);
>      debug (h,
> -           "enter: buf=\"%s\" count=%zu
offset=%"PRIu64" completion=%s flags=0x%x", buf_printable ?
buf_printable : "", count, offset, CALLBACK_IS_NULL
(completion_callback) ? "<fun>" : "NULL", flags);
> +           "enter: buf=\"%s\" count=%zu
offset=%"PRIu64" completion=%s "
> +           "flags=0x%x",
> +           buf_printable ? buf_printable : "", count, offset,
> +           CALLBACK_IS_NULL (completion_callback) ?
"<fun>" : "NULL", flags);
>      free (buf_printable);
>    }
>
The second longest one [lib/api.c]:
> @@ -5394,7 +5470,10 @@ nbd_aio_pread_structured (struct nbd_han
>    pthread_mutex_lock (&h->lock);
>    if_debug (h) {
>      debug (h,
> -           "enter: buf=<buf> count=%zu
offset=%"PRIu64" chunk=%s completion=%s flags=0x%x", count,
offset, "<fun>", CALLBACK_IS_NULL (completion_callback) ?
"<fun>" : "NULL", flags);
> +           "enter: buf=<buf> count=%zu
offset=%"PRIu64" chunk=%s "
> +           "completion=%s flags=0x%x",
> +           count, offset, "<fun>",
> +           CALLBACK_IS_NULL (completion_callback) ?
"<fun>" : "NULL", flags);
>    }
>
>    if (h->pread_initialize)
There is one debug_direct("enter") invocation change [lib/api.c]:
> @@ -332,7 +335,8 @@ nbd_set_private_data (struct nbd_handle
>    /* This function must not call set_error. */
>    if_debug (h) {
>      debug_direct (h, "nbd_set_private_data",
> -                  "enter: private_data=%"PRIuPTR"",
private_data);
> +                  "enter: private_data=%"PRIuPTR"",
> +                  private_data);
>    }
>
>    ret = nbd_unlocked_set_private_data (h, private_data);
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 at redhat.com>
---
 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

Laszlo Ersek

2023-Apr-25 07:11 UTC

head link

[Libguestfs] [libnbd PATCH 17/23] generator/C: print_wrapper: break apart the "ready" & "processing" states

Factor out the " ||\n        " separator string from the
"String.concat"
call, and reuse it within the Connected state mapping.

An example of the effect [lib/api.c]:
> @@ -625,7 +625,8 @@ get_canonical_export_name_in_permitted_s
>    const enum state state = get_public_state (h);
>
>    if (!(nbd_internal_is_state_negotiating (state) ||
> -        nbd_internal_is_state_ready (state) ||
nbd_internal_is_state_processing (state) ||
> +        nbd_internal_is_state_ready (state) ||
> +        nbd_internal_is_state_processing (state) ||
>          nbd_internal_is_state_closed (state))) {
>      set_error (nbd_internal_is_state_created (state) ? ENOTCONN : EINVAL,
>                 "invalid state: %s: the handle must be %s",
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 generator/C.ml | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/generator/C.ml b/generator/C.ml
index ce6ff69419a5..f7a306de39d8 100644
--- a/generator/C.ml
+++ b/generator/C.ml
@@ -533,17 +533,21 @@ let
       pr "{\n";
       pr "  const enum state state = get_public_state (h);\n";
       pr "\n";
+      let or_nl_indent = " ||\n        " in
       let tests          List.map (
           function
           | Created -> "nbd_internal_is_state_created (state)"
           | Connecting -> "nbd_internal_is_state_connecting
(state)"
           | Negotiating -> "nbd_internal_is_state_negotiating
(state)"
-          | Connected -> "nbd_internal_is_state_ready (state) ||
nbd_internal_is_state_processing (state)"
+          | Connected ->
+              "nbd_internal_is_state_ready (state)" ^
+              or_nl_indent ^
+              "nbd_internal_is_state_processing (state)"
           | Closed -> "nbd_internal_is_state_closed (state)"
           | Dead -> "nbd_internal_is_state_dead (state)"
         ) permitted_states in
-      pr "  if (!(%s)) {\n" (String.concat " ||\n        "
tests);
+      pr "  if (!(%s)) {\n" (String.concat or_nl_indent tests);
       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";

Laszlo Ersek

2023-Apr-25 07:11 UTC

head link

[Libguestfs] [libnbd PATCH 18/23] generator/C: print_wrapper: fold permitted states error message

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]:
> @@ -6522,7 +6522,8 @@ get_uri_in_permitted_state (struct nbd_h
>      set_error (nbd_internal_is_state_created (state) ? ENOTCONN : EINVAL,
>                 "invalid state: %s: the handle must be %s",
>                 nbd_internal_state_short_string (state),
> -               "connecting, or negotiating, or connected with the
server, or shut down, or dead");
> +               "connecting, or negotiating, or connected with the
server, "
> +               "or shut down, or dead");
>      return false;
>    }
>    return true;
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 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";

Laszlo Ersek

2023-Apr-25 07:11 UTC

head link

[Libguestfs] [libnbd PATCH 19/23] generator/C: print_trace_leave: separate arg list from function designator

Dependent on "may_set_error", we (a) call debug() vs. debug_direct(),
(b)
don't pass vs. pass the API name to the function from (a).

Currently this is (justifiedly) handled within a single "if"; however,
for
wrapping the argument lists later -- with differrent indentations --,
let's now split the arg lists to separate invocations of "pr", at
the cost
of another "if may_set_error".

This is a refactoring; it does not change the generated output.

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

diff --git a/generator/C.ml b/generator/C.ml
index 17b22d84d07d..7c8fffd89435 100644
--- a/generator/C.ml
+++ b/generator/C.ml
@@ -854,11 +854,14 @@ let
           pr "          nbd_internal_printable_string (ret);\n"
        | _ -> ()
       );
-      pr "      debug (h, \"leave: ret="
+      pr "      debug ("
     )
-    else (
-      pr "    debug_direct (h, \"nbd_%s\", \"leave:
ret=" name
-    );
+    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"

Laszlo Ersek

2023-Apr-25 07:11 UTC

head link

[Libguestfs] [libnbd PATCH 20/23] generator/C: print_trace_leave: wrap "leave" debug() / debug_direct() call

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]:
> @@ -247,7 +247,8 @@ nbd_stats_chunks_received (struct nbd_ha
>    ret = nbd_unlocked_stats_chunks_received (h);
>
>    if_debug (h) {
> -    debug_direct (h, "nbd_stats_chunks_received", "leave:
ret=%" PRIu64, ret);
> +    debug_direct (h, "nbd_stats_chunks_received", "leave:
ret=%" PRIu64,
> +                  ret);
>    }
>
>    if (h->public_state != get_next_state (h))
> @@ -1211,7 +1212,8 @@ nbd_get_request_structured_replies (stru
>    ret = nbd_unlocked_get_request_structured_replies (h);
>
>    if_debug (h) {
> -    debug_direct (h, "nbd_get_request_structured_replies",
"leave: ret=%d", ret);
> +    debug_direct (h, "nbd_get_request_structured_replies",
"leave: ret=%d",
> +                  ret);
>    }
>
>    if (h->public_state != get_next_state (h))
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 at redhat.com>
---
 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

Laszlo Ersek

2023-Apr-25 07:11 UTC

head link

[Libguestfs] [libnbd PATCH 21/23] generator/C: print_flags_check: break guard condition to a new line

Break the flags guard condition, if any, to a new line.

One of the most impactful changes of this patch is [lib/api.c]:
> @@ -4218,7 +4224,8 @@ nbd_zero (struct nbd_handle *h, uint64_t
>      ret = -1;
>      goto out;
>    }
> -  if (unlikely ((flags & ~0x13) != 0) && ((h->strict &
LIBNBD_STRICT_FLAGS) || flags > UINT16_MAX)) {
> +  if (unlikely ((flags & ~0x13) != 0) &&
> +      ((h->strict & LIBNBD_STRICT_FLAGS) || flags > UINT16_MAX))
{
>      set_error (EINVAL, "%s: invalid value for flag: 0x%x",
>                 "flags", flags);
>      ret = -1;
Note that this patch is not really future-proof; if we add another

  guard = Some "...";

to "generator/API.ml", that string can still exceed 80 characters.

For now, the current patch should suffice.

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

diff --git a/generator/C.ml b/generator/C.ml
index cff8f854f1d1..cf44424517de 100644
--- a/generator/C.ml
+++ b/generator/C.ml
@@ -622,7 +622,7 @@ let
            sprintf "0x%x" !v
         | None -> "LIBNBD_" ^ flag_prefix ^ "_MASK" in
       let guard = match guard with
-        | Some value -> " && " ^ value
+        | Some value -> " &&\n      " ^ value
         | None -> "" in
       pr "  if (unlikely ((%s & ~%s) != 0)%s) {\n" n mask guard;
       pr "    set_error (EINVAL, \"%%s: invalid value for flag:
0x%%x\",\n";

Laszlo Ersek

2023-Apr-25 07:11 UTC

head link

[Libguestfs] [libnbd PATCH 22/23] generator/C: print_wrapper: use helper variable for permitted state check

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]:
> @@ -4805,7 +4943,9 @@ nbd_aio_connect_systemd_socket_activatio
>      free (argv_printable);
>    }
>
> -  if (unlikely (!aio_connect_systemd_socket_activation_in_permitted_state
(h))) {
> +  bool p;
> +  p = aio_connect_systemd_socket_activation_in_permitted_state (h);
> +  if (unlikely (!p)) {
>      ret = -1;
>      goto out;
>    }
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 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";

Laszlo Ersek

2023-Apr-25 07:11 UTC

head link

[Libguestfs] [libnbd PATCH 23/23] generator/C: lib/api.c: indent arg list 2 spaces relative to function name

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]:
> @@ -5378,9 +5577,11 @@ aio_opt_list_meta_context_queries_in_per
>  }
>
>  int
> -nbd_aio_opt_list_meta_context_queries (struct nbd_handle *h, char
**queries,
> -                                       nbd_context_callback
context_callback,
> -                                       nbd_completion_callback
completion_callback)
> +nbd_aio_opt_list_meta_context_queries (
> +  struct nbd_handle *h, char **queries,
> +  nbd_context_callback context_callback,
> +  nbd_completion_callback completion_callback
> +)
>  {
>    int ret;
>
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 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;

Libguestfs - Apr 2023 - [libnbd PATCH 22/23] generator/C: print_wrapper: use helper variable for permitted state check

[Libguestfs] [libnbd PATCH 00/23] continue wrapping generated C code harder

[Libguestfs] [libnbd PATCH 01/23] generator: split LIBNBD_ATTRIBUTE_NONNULL replacement text

[Libguestfs] [libnbd PATCH 02/23] generator: wrap "python/methods.h" at 80 characters

[Libguestfs] [libnbd PATCH 03/23] generator: wrap "python/libnbdmod.c" at 80 characters

[Libguestfs] [libnbd PATCH 04/23] generator: split "int64_from_uint32_array" calls in "ocaml/nbd-c.c"

[Libguestfs] [libnbd PATCH 05/23] generator: wrap nbd_internal_ocaml_raise_closed() calls in "ocaml/nbd-c.c"

[Libguestfs] [libnbd PATCH 06/23] generator: factor out "num_args" in "print_ocaml_binding"

[Libguestfs] [libnbd PATCH 07/23] generator: wrap byte-code compat function bodies in "ocaml/nbd-c.c"

[Libguestfs] [libnbd PATCH 08/23] generator/utils: factor out "pr_buf"

[Libguestfs] [libnbd PATCH 09/23] generator/utils: comment on "maxcol" handling in "pr_wrap"

[Libguestfs] [libnbd PATCH 10/23] generator/utils: add "pr_wrap_cstr"

[Libguestfs] [libnbd PATCH 11/23] generator/C: print_trace_enter: break "enter" to a common new line

[Libguestfs] [libnbd PATCH 12/23] generator/C: print_trace_enter: expand "function" shorthand in value iters

[Libguestfs] [libnbd PATCH 13/23] generator/C: print_trace_enter: print Closure values uniformly with others

[Libguestfs] [libnbd PATCH 14/23] generator/C: print_trace_enter: generate commas as suffixes, not prefixes

[Libguestfs] [libnbd PATCH 15/23] generator/C: print_trace_enter: strip spaces around PRI* macros

[Libguestfs] [libnbd PATCH 16/23] generator/C: print_trace_enter: wrap "enter" debug() / debug_direct() call

[Libguestfs] [libnbd PATCH 17/23] generator/C: print_wrapper: break apart the "ready" & "processing" states

[Libguestfs] [libnbd PATCH 18/23] generator/C: print_wrapper: fold permitted states error message

[Libguestfs] [libnbd PATCH 19/23] generator/C: print_trace_leave: separate arg list from function designator

[Libguestfs] [libnbd PATCH 20/23] generator/C: print_trace_leave: wrap "leave" debug() / debug_direct() call

[Libguestfs] [libnbd PATCH 21/23] generator/C: print_flags_check: break guard condition to a new line

[Libguestfs] [libnbd PATCH 22/23] generator/C: print_wrapper: use helper variable for permitted state check

[Libguestfs] [libnbd PATCH 23/23] generator/C: lib/api.c: indent arg list 2 spaces relative to function name