thr3ads.net - Libguestfs - [Libguestfs] [libnbd PATCH 2/6] generator/utils: add "pr_wrap_c

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

Laszlo Ersek

2023-May-10 11:48 UTC

[Libguestfs] [libnbd PATCH 0/6] finish wrapping generated C code harder

This is the last wave (wave 5). Line length maxima:

  file              before  after
  ----------------  ------  -----
  lib/states-run.c     102     98
  lib/states.c         116     80
  lib/states.h         123     86

The longest line in "lib/states.h" becomes:
> extern int
nbd_internal_enter_STATE_NEWSTYLE_OPT_STRUCTURED_REPLY_RECV_REPLY_PAYLOAD (
>              struct nbd_handle *h, bool *blocked
>            );
The one in "lib/states-run.c":
>     case STATE_NEWSTYLE_OPT_STRUCTURED_REPLY_RECV_REPLY_PAYLOAD:
>       r =
nbd_internal_enter_STATE_NEWSTYLE_OPT_STRUCTURED_REPLY_RECV_REPLY_PAYLOAD (h,
&blocked);
>       break;
We couldn't find a "scalable" approach for shortening these (the
state
machine can be nested indefinitely deeply), so we decided to live with
them.

Thanks
Laszlo
Laszlo Ersek (6):
  state_machine_generator: wrap debug() calls in nbd_internal_run()
  generator/utils: add "pr_wrap_c_comment"
  state_machine_generator: wrap state comments in lib/states.{h,c}
  state_machine_generator: wrap nbd_internal_enter_* prototypes
  state_machine_generator: wrap enter_*() calls and prototypes
  state_machine_generator: rename, and break up the init. of,
    "next_state"

 generator/state_machine_generator.ml | 48 +++++++----
 generator/utils.ml                   | 87 ++++++++++++++++++++
 generator/utils.mli                  |  1 +
 3 files changed, 119 insertions(+), 17 deletions(-)


base-commit: 99d18799f4079b9be073a3d1b0b9bf5beccf0dff

Laszlo Ersek

2023-May-10 11:48 UTC

head link

[Libguestfs] [libnbd PATCH 1/6] state_machine_generator: wrap debug() calls in nbd_internal_run()

The longest line in "lib/states-run.c" is folded as follows:
> @@ -513,8 +515,9 @@ nbd_internal_run (struct nbd_handle *h,
>        goto ok;
>      case notify_read:
>        set_next_state (h, STATE_ISSUE_COMMAND_PAUSE_WRITE_SHUTDOWN);
> -      debug (h, "event %s: %s -> %s",
> -             "NotifyRead",
"ISSUE_COMMAND.SEND_WRITE_SHUTDOWN",
"ISSUE_COMMAND.PAUSE_WRITE_SHUTDOWN");
> +      debug (h, "event %s: %s -> %s", "NotifyRead",
> +             "ISSUE_COMMAND.SEND_WRITE_SHUTDOWN",
> +             "ISSUE_COMMAND.PAUSE_WRITE_SHUTDOWN");
>        goto ok;
>      default: ; /* nothing, silence GCC warning */
>      }
Note: this is the last (aka "only") wrapping-related patch for
"lib/states-run.c". We still have overlong lines, but those come from
generated single-identifier names such as
nbd_internal_enter_STATE_NEWSTYLE_OPT_STRUCTURED_REPLY_RECV_REPLY_PAYLOAD().
After discussing those with Rich and Eric, we're going to leave them
alone.

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

diff --git a/generator/state_machine_generator.ml
b/generator/state_machine_generator.ml
index 43c1ce4c14dc..3df706f91ccd 100644
--- a/generator/state_machine_generator.ml
+++ b/generator/state_machine_generator.ml
@@ -414,10 +414,15 @@ let
             pr "    case %s:\n" (c_string_of_external_event e);
             if state != next_state then (
               pr "      set_next_state (h, %s);\n"
next_state.parsed.state_enum;
-              pr "      debug (h, \"event %%s: %%s ->
%%s\",\n";
-              pr "             \"%s\", \"%s\",
\"%s\");\n"
-                 (string_of_external_event e)
-                 display_name next_state.parsed.display_name;
+              pr "      debug (";
+              let print_debug_args () +                pr "h, \"event
%%s: %%s -> %%s\", ";
+                pr "\"%s\", \"%s\",
\"%s\");"
+                   (string_of_external_event e)
+                   display_name next_state.parsed.display_name;
+              in
+              pr_wrap ',' print_debug_args;
+              pr "\n"
             );
             pr "      goto ok;\n";
         ) events;

Laszlo Ersek

2023-May-10 11:48 UTC

head link

[Libguestfs] [libnbd PATCH 2/6] generator/utils: add "pr_wrap_c_comment"

Add the "pr_wrap_c_comment" function, for wrapping C comments.

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  | 87 ++++++++++++++++++++
 2 files changed, 88 insertions(+)

diff --git a/generator/utils.mli b/generator/utils.mli
index 9d6640c87842..e622e318c2b7 100644
--- a/generator/utils.mli
+++ b/generator/utils.mli
@@ -54,6 +54,7 @@ val
 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 pr_wrap_c_comment : ?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 ff1e8a72f7aa..8f9ce8a84910 100644
--- a/generator/utils.ml
+++ b/generator/utils.ml
@@ -267,6 +267,93 @@ let
        ) fields
   | _ -> assert false
 
+(* Wrap a string as a (potentially multi-line) C comment. Two things to note:
+ * - the function produces both the starting slash-star and the ending
+ *   star-slash,
+ * - newline characters in the input are not allowed.
+ *)
+let pr_wrap_c_comment ?(maxcol = 80) code +  (* The comment delimiters. *)
+  let start = "/* "
+  and sep   = " * "
+  and stop  = " */"
+
+  (* Format the comment into a buffer, and append a space character, for
forcing
+   * a nonspace -> space transiton at the end of the comment, provided the
+   * comment ends with a nonspace. Note that trailing spaces will be swallowed
+   * anyway, as only nonspace -> space transitions produce output.
+   *)
+  and comment = pr_buf (fun () -> code (); pr " ")
+
+  (* Capture the current column / indentation. *)
+  and indent = spaces !col
+
+  (* Whether we're currently scanning spaces. We start the loop under the
+   * assumption "scanning spaces" because a space -> nonspace
transition does
+   * not try to output anything.
+   *)
+  and scanning_spaces = ref true
+
+  (* The "buffers" for accumulating spaces and nonspaces. *)
+  and spaces_start = ref 0
+  and nonspaces_start = ref 0
+
+  (* Whether we've needed to insert at least one line break. *)
+  and multiline = ref false in
+
+  pr "%s" start;
+
+  for i = 0 to Buffer.length comment - 1 do
+    let ch = Buffer.nth comment i in
+
+    (* Newlines are invalid... *)
+    assert (ch <> '\n');
+
+    match !scanning_spaces, ch with
+    | true, ' ' ->
+        ()
+    | true, _ ->
+        (* Space -> nonspace transition. *)
+        scanning_spaces := false;
+        nonspaces_start := i
+    | false, ' ' ->
+        (* Nonspace -> space transition. If the buffered spaces:
+         *
+         *   nonspaces_start - spaces_start
+         *
+         * plus the buffered nonspaces:
+         *
+         *   i - nonspaces_start
+         *
+         * fit on the current line, then print both buffers. (Note that the sum
+         * of those addends is just (i - spaces_start).)
+         *
+         * Otherwise, insert a line break and a comment line separator, and
only
+         * print the nonspaces.
+         *)
+        if !col + (i - !spaces_start) <= maxcol then
+          pr "%s" (Buffer.sub comment !spaces_start (i -
!spaces_start))
+        else (
+          pr "\n%s%s%s" indent sep
+            (Buffer.sub comment !nonspaces_start (i - !nonspaces_start));
+          multiline := true
+        );
+
+        scanning_spaces := true;
+        spaces_start := i
+    | false, _ ->
+        ()
+  done;
+
+  (* If the comment has fit on a single line, and we've got room left for
the
+   * terminator, then place the terminator on the same line. Otherwise, break
+   * the terminator to a new line.
+   *)
+  if not !multiline && !col + String.length stop <= maxcol then
+    pr "%s" stop
+  else
+    pr "\n%s%s" indent stop
+
 let output_lineno () = !lineno
 let output_column () = !col

Laszlo Ersek

2023-May-10 11:48 UTC

head link

[Libguestfs] [libnbd PATCH 3/6] state_machine_generator: wrap state comments in lib/states.{h, c}

Wrap those comments in "lib/states.h" and "lib/states.c"
that describe the
automaton's states.

Example changes from "lib/states.h":
>    /* CONNECT_TCP.CONNECT: Initial call to connect(2) on a TCP socket */
>    STATE_CONNECT_TCP_CONNECT,
>
> -  /* CONNECT_TCP.CONNECTING: Connecting to the remote server over a TCP
socket */
> +  /* CONNECT_TCP.CONNECTING: Connecting to the remote server over a TCP
socket
> +   */
>    STATE_CONNECT_TCP_CONNECTING,
>
and
>    STATE_NEWSTYLE_OPT_META_CONTEXT_SEND_QUERY,
>
> -  /* NEWSTYLE.OPT_META_CONTEXT.PREPARE_FOR_REPLY: Prepare to receive
newstyle NBD_OPT_SET_META_CONTEXT option reply */
> +  /* NEWSTYLE.OPT_META_CONTEXT.PREPARE_FOR_REPLY: Prepare to receive
newstyle
> +   * NBD_OPT_SET_META_CONTEXT option reply
> +   */
>    STATE_NEWSTYLE_OPT_META_CONTEXT_PREPARE_FOR_REPLY,
>
Example changes from "lib/states.c":
> @@ -2085,7 +2087,8 @@ nbd_internal_enter_STATE_NEWSTYLE_OPT_ST
>    return r;
>  }
>
> -/* NEWSTYLE.OPT_STARTTLS.SEND: Send newstyle NBD_OPT_STARTTLS to upgrade
to TLS */
> +/* NEWSTYLE.OPT_STARTTLS.SEND: Send newstyle NBD_OPT_STARTTLS to upgrade
to TLS
> + */
>  static int
>  enter_STATE_NEWSTYLE_OPT_STARTTLS_SEND (struct nbd_handle *h,
>               enum state *next_state,
and
> @@ -2992,7 +3021,9 @@ nbd_internal_enter_STATE_NEWSTYLE_OPT_ME
>    return r;
>  }
>
> -/* NEWSTYLE.OPT_META_CONTEXT.PREPARE_FOR_REPLY: Prepare to receive
newstyle NBD_OPT_SET_META_CONTEXT option reply */
> +/* NEWSTYLE.OPT_META_CONTEXT.PREPARE_FOR_REPLY: Prepare to receive
newstyle
> + * NBD_OPT_SET_META_CONTEXT option reply
> + */
>  static int
>  enter_STATE_NEWSTYLE_OPT_META_CONTEXT_PREPARE_FOR_REPLY (struct nbd_handle
*h,
>               enum state *next_state,
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 generator/state_machine_generator.ml | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/generator/state_machine_generator.ml
b/generator/state_machine_generator.ml
index 3df706f91ccd..74a009a33598 100644
--- a/generator/state_machine_generator.ml
+++ b/generator/state_machine_generator.ml
@@ -298,7 +298,9 @@ let
   pr "enum state {\n";
   List.iter (
     fun ({ comment; parsed = { display_name; state_enum } }) ->
-      pr "  /* %s: %s */\n" display_name comment;
+      pr "  ";
+      pr_wrap_c_comment (fun () -> pr "%s: %s" display_name
comment);
+      pr "\n";
       pr "  %s,\n" state_enum;
       pr "\n";
   ) states;
@@ -346,7 +348,8 @@ let
   List.iter (
     fun { comment; parsed = { display_name; state_enum; loc; code } } ->
       pr "\n";
-      pr "/* %s: %s */\n" display_name comment;
+      pr_wrap_c_comment (fun () -> pr "%s: %s" display_name
comment);
+      pr "\n";
       pr "static int\n";
       pr "enter_%s (struct nbd_handle *h,\n" state_enum;
       pr "             enum state *next_state,\n";

Laszlo Ersek

2023-May-10 11:48 UTC

head link

[Libguestfs] [libnbd PATCH 4/6] state_machine_generator: wrap nbd_internal_enter_* prototypes

Similarly to commit bfef979aa372 ("generator: wrap
"python/methods.h" at
80 characters", 2023-05-02), just break the fixed (and in itself, short)
parameter list to a new line.

Example effect in "lib/states.h":
> -extern int
nbd_internal_enter_STATE_NEWSTYLE_OPT_STRUCTURED_REPLY_RECV_REPLY_PAYLOAD
(struct nbd_handle *h, bool *blocked);
> +extern int
nbd_internal_enter_STATE_NEWSTYLE_OPT_STRUCTURED_REPLY_RECV_REPLY_PAYLOAD (
> +             struct nbd_handle *h, bool *blocked
> +           );
Example effect in "lib/states.c":
> @@ -2566,9 +2626,11 @@ enter_STATE_NEWSTYLE_OPT_STRUCTURED_REPL
>
>  }
>
> -#line 2570 "lib/states.c"
> +#line 2630 "lib/states.c"
>  int
> -nbd_internal_enter_STATE_NEWSTYLE_OPT_STRUCTURED_REPLY_RECV_REPLY_PAYLOAD
(struct nbd_handle *h, bool *blocked)
> +nbd_internal_enter_STATE_NEWSTYLE_OPT_STRUCTURED_REPLY_RECV_REPLY_PAYLOAD
(
> +  struct nbd_handle *h, bool *blocked
> +)
>  {
>    int r;
>    enum state next_state =
STATE_NEWSTYLE_OPT_STRUCTURED_REPLY_RECV_REPLY_PAYLOAD;
Note that some of the modified lines remain longer than 80 characters, but
we'll live with that, as we don't want to change the
"state_enum" field.

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

diff --git a/generator/state_machine_generator.ml
b/generator/state_machine_generator.ml
index 74a009a33598..8667adeacf7a 100644
--- a/generator/state_machine_generator.ml
+++ b/generator/state_machine_generator.ml
@@ -333,7 +333,9 @@ let
   pr "/* State transitions defined in states.c. */\n";
   List.iter (
     fun { parsed = { state_enum } } ->
-      pr "extern int nbd_internal_enter_%s (struct nbd_handle *h, bool
*blocked);\n" state_enum;
+      pr "extern int nbd_internal_enter_%s (\n" state_enum;
+      pr "             struct nbd_handle *h, bool *blocked\n";
+      pr "           );\n"
   ) states
 
 let generate_lib_states_c () @@ -366,8 +368,9 @@ let
       let output_loc = "lib/states.c", output_lineno () + 1 in
       pr "%s\n" (line_directive_of_location output_loc);
       pr "int\n";
-      pr "nbd_internal_enter_%s (struct nbd_handle *h, bool
*blocked)\n"
-        state_enum;
+      pr "nbd_internal_enter_%s (\n" state_enum;
+      pr "  struct nbd_handle *h, bool *blocked\n";
+      pr ")\n";
       pr "{\n";
       pr "  int r;\n";
       pr "  enum state next_state = %s;\n" state_enum;

Laszlo Ersek

2023-May-10 11:48 UTC

head link

[Libguestfs] [libnbd PATCH 5/6] state_machine_generator: wrap enter_*() calls and prototypes

More of the same.

Regarding the enter_*() calls, most impactful change is [lib/states.c]:
> @@ -2635,7 +2695,9 @@ nbd_internal_enter_STATE_NEWSTYLE_OPT_ST
>    int r;
>    enum state next_state =
STATE_NEWSTYLE_OPT_STRUCTURED_REPLY_RECV_REPLY_PAYLOAD;
>
> -  r = enter_STATE_NEWSTYLE_OPT_STRUCTURED_REPLY_RECV_REPLY_PAYLOAD (h,
&next_state, blocked);
> +  r = enter_STATE_NEWSTYLE_OPT_STRUCTURED_REPLY_RECV_REPLY_PAYLOAD (
> +        h, &next_state, blocked
> +      );
>    if (get_next_state (h) != next_state) {
>      debug (h, "transition: %s -> %s",
>             "NEWSTYLE.OPT_STRUCTURED_REPLY.RECV_REPLY_PAYLOAD",
(Note that using "pr_wrap" instead of the fixed manual wrapping would
have
both advantages and disadvantages:

- Many state paths (formatted with underscores in "state_name") are
  actually short; "pr_wrap" would keep those enter_*() calls
unchanged.

- A few state paths are extremely long however, and such a path is
  generally unbounded. For example, even the opening parenthesis could be
  in column 80+. That would make "pr_wrap" entirely useless.)

While at it, update the existent manual wrapping for the enter_*()
prototypes as well; that makes the calls and the prototypes look uniform,
plus it actually removes at least one overlong line [lib/states.c]:
> @@ -2612,9 +2672,9 @@ nbd_internal_enter_STATE_NEWSTYLE_OPT_ST
>   * NBD_OPT_STRUCTURED_REPLY reply payload
>   */
>  static int
> -enter_STATE_NEWSTYLE_OPT_STRUCTURED_REPLY_RECV_REPLY_PAYLOAD (struct
nbd_handle *h,
> -             enum state *next_state,
> -             bool *blocked)
> +enter_STATE_NEWSTYLE_OPT_STRUCTURED_REPLY_RECV_REPLY_PAYLOAD (
> +  struct nbd_handle *h, enum state *next_state, bool *blocked
> +)
>  {
>  #line 68 "generator/states-newstyle-opt-structured-reply.c"
>
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 generator/state_machine_generator.ml | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/generator/state_machine_generator.ml
b/generator/state_machine_generator.ml
index 8667adeacf7a..af07e4ff3966 100644
--- a/generator/state_machine_generator.ml
+++ b/generator/state_machine_generator.ml
@@ -353,9 +353,9 @@ let
       pr_wrap_c_comment (fun () -> pr "%s: %s" display_name
comment);
       pr "\n";
       pr "static int\n";
-      pr "enter_%s (struct nbd_handle *h,\n" state_enum;
-      pr "             enum state *next_state,\n";
-      pr "             bool *blocked)\n";
+      pr "enter_%s (\n" state_enum;
+      pr "  struct nbd_handle *h, enum state *next_state, bool
*blocked\n";
+      pr ")\n";
       pr "{\n";
       if code <> "" then (
         pr "%s\n" (line_directive_of_location loc);
@@ -375,7 +375,9 @@ let
       pr "  int r;\n";
       pr "  enum state next_state = %s;\n" state_enum;
       pr "\n";
-      pr "  r = enter_%s (h, &next_state, blocked);\n"
state_enum;
+      pr "  r = enter_%s (\n" state_enum;
+      pr "        h, &next_state, blocked\n";
+      pr "      );\n";
       pr "  if (get_next_state (h) != next_state) {\n";
       pr "    debug (h, \"transition: %%s -> %%s\",\n";
       pr "           \"%s\",\n" display_name;

Laszlo Ersek

2023-May-10 11:48 UTC

head link

[Libguestfs] [libnbd PATCH 6/6] state_machine_generator: rename, and break up the init. of, "next_state"

We can shave off 17 characters by renaming "next_state" to
"next", and
replacing its initialization with a standalone assignment.

Most impactful effect [lib/states.c]:
>  int
>  nbd_internal_enter_STATE_NEWSTYLE_OPT_STRUCTURED_REPLY_RECV_REPLY_PAYLOAD
(
>    struct nbd_handle *h, bool *blocked
>  )
>  {
>    int r;
> -  enum state next_state =
STATE_NEWSTYLE_OPT_STRUCTURED_REPLY_RECV_REPLY_PAYLOAD;
> +  enum state next;
>
> +  next = STATE_NEWSTYLE_OPT_STRUCTURED_REPLY_RECV_REPLY_PAYLOAD;
>    r = enter_STATE_NEWSTYLE_OPT_STRUCTURED_REPLY_RECV_REPLY_PAYLOAD (
> -        h, &next_state, blocked
> +        h, &next, blocked
>        );
> -  if (get_next_state (h) != next_state) {
> +  if (get_next_state (h) != next) {
>      debug (h, "transition: %s -> %s",
>             "NEWSTYLE.OPT_STRUCTURED_REPLY.RECV_REPLY_PAYLOAD",
> -           nbd_internal_state_short_string (next_state));
> -    set_next_state (h, next_state);
> +           nbd_internal_state_short_string (next));
> +    set_next_state (h, next);
>    }
>    return r;
>  }
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 generator/state_machine_generator.ml | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/generator/state_machine_generator.ml
b/generator/state_machine_generator.ml
index af07e4ff3966..274e290952c1 100644
--- a/generator/state_machine_generator.ml
+++ b/generator/state_machine_generator.ml
@@ -373,16 +373,17 @@ let
       pr ")\n";
       pr "{\n";
       pr "  int r;\n";
-      pr "  enum state next_state = %s;\n" state_enum;
+      pr "  enum state next;\n";
       pr "\n";
+      pr "  next = %s;\n" state_enum;
       pr "  r = enter_%s (\n" state_enum;
-      pr "        h, &next_state, blocked\n";
+      pr "        h, &next, blocked\n";
       pr "      );\n";
-      pr "  if (get_next_state (h) != next_state) {\n";
+      pr "  if (get_next_state (h) != next) {\n";
       pr "    debug (h, \"transition: %%s -> %%s\",\n";
       pr "           \"%s\",\n" display_name;
-      pr "           nbd_internal_state_short_string
(next_state));\n";
-      pr "    set_next_state (h, next_state);\n";
+      pr "           nbd_internal_state_short_string (next));\n";
+      pr "    set_next_state (h, next);\n";
       pr "  }\n";
       pr "  return r;\n";
       pr "}\n";

Eric Blake

2023-May-10 15:25 UTC

head link

[Libguestfs] [libnbd PATCH 0/6] finish wrapping generated C code harder

On Wed, May 10, 2023 at 01:48:08PM +0200, Laszlo Ersek
wrote:> This is the last wave (wave 5). Line length maxima:
> 
>   file              before  after
>   ----------------  ------  -----
>   lib/states-run.c     102     98
>   lib/states.c         116     80
>   lib/states.h         123     86
> 
> The longest line in "lib/states.h" becomes:
> 
> > extern int
nbd_internal_enter_STATE_NEWSTYLE_OPT_STRUCTURED_REPLY_RECV_REPLY_PAYLOAD (
> >              struct nbd_handle *h, bool *blocked
> >            );
> 
> The one in "lib/states-run.c":
> 
> >     case STATE_NEWSTYLE_OPT_STRUCTURED_REPLY_RECV_REPLY_PAYLOAD:
> >       r =
nbd_internal_enter_STATE_NEWSTYLE_OPT_STRUCTURED_REPLY_RECV_REPLY_PAYLOAD (h,
&blocked);
> >       break;
> 
> We couldn't find a "scalable" approach for shortening these
(the state
> machine can be nested indefinitely deeply), so we decided to live with
> them.
For public reference, we considered approaches such as shortening
specific state names:

s/NEWSTYLE/NEW/
s/OPT_STRUCTURED_REPLY/OPT_SR/
s/RECV_REPLY_PAYLOAD/RECV_PAYLOAD/

but the more names we change, the more places we have to touch, and it
doesn't alleviate the possibility of future state names being long.

We also considered a different way of writing state names, by
introducing a series of glue macros, where you can then wrap macro
parameters but still generate a long C identifier, something like:

extern int NBD_GLUE4(nbd_internal_enter_STATE, NEWSTYLE,
                     OPT_STRUCTURED_REPLY, RECV_REPLY_PAYLOAD) {
...
    case NBD_GLUE4(STATE, NEWSTYLE, OPT_STRUCTURED_REPLY,
                   RECV_REPLY_PAYLOAD):

with variations such as writing a variadic macro NBD_GLUE(...) using
__VA_ARGS__ that then dispatches to the correct NBD_GLUE<n> rather
than having to be explicit about it at callsites.  But that hurts
grep'ability when stepping through the state machine in gdb.
> 
> Thanks
> Laszlo
> Laszlo Ersek (6):
>   state_machine_generator: wrap debug() calls in nbd_internal_run()
>   generator/utils: add "pr_wrap_c_comment"
>   state_machine_generator: wrap state comments in lib/states.{h,c}
>   state_machine_generator: wrap nbd_internal_enter_* prototypes
>   state_machine_generator: wrap enter_*() calls and prototypes
>   state_machine_generator: rename, and break up the init. of,
>     "next_state"
For those without explicit comments,

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

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

Richard W.M. Jones

2023-May-15 11:21 UTC

head link

[Libguestfs] [libnbd PATCH 0/6] finish wrapping generated C code harder

It all looks fine to me, barring comments that Eric has already made,
so ACK series.

Thanks,

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

Libguestfs - May 2023 - [libnbd PATCH 2/6] generator/utils: add "pr_wrap_c_comment"

[Libguestfs] [libnbd PATCH 0/6] finish wrapping generated C code harder

[Libguestfs] [libnbd PATCH 1/6] state_machine_generator: wrap debug() calls in nbd_internal_run()

[Libguestfs] [libnbd PATCH 2/6] generator/utils: add "pr_wrap_c_comment"

[Libguestfs] [libnbd PATCH 3/6] state_machine_generator: wrap state comments in lib/states.{h, c}

[Libguestfs] [libnbd PATCH 4/6] state_machine_generator: wrap nbd_internal_enter_* prototypes

[Libguestfs] [libnbd PATCH 5/6] state_machine_generator: wrap enter_*() calls and prototypes

[Libguestfs] [libnbd PATCH 6/6] state_machine_generator: rename, and break up the init. of, "next_state"

[Libguestfs] [libnbd PATCH 0/6] finish wrapping generated C code harder

[Libguestfs] [libnbd PATCH 0/6] finish wrapping generated C code harder