Libguestfs - Apr 2023 - [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";

On Tue, Apr 25, 2023 at 09:11:06AM +0200, Laszlo Ersek
wrote:
> 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);
Declaration after statement.  I'm not opposed to it here, but it
doesn't seem to be our prevailing style, and can cause issues
elsewhere (any function with a goto or switch that jumps past a
declaration can cause weird effects for using an uninitialized
variable even when the declaration had an initializer; gcc in turn has
warnings options that catches those but then flags other late
declarations as suspicious).

Should we hoist the declaration of p earlier in the generated
function?

Otherwise, the rest of this series looks good to me.  You may want to
wait for Rich's review, given the amount of OCaml involved (I may have
missed something), but the generated code does look nicer after this
series.

Feel free to add
Reviewed-by: Eric Blake <eblake at rehat.com>
throughout the series when pushing.

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