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

[Laszlo Ersek]

On 4/26/23 22:25, Eric Blake wrote:
> 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.
> 

I've pushed the first 21 patches as commit range
0744f748ed90..2fa6198f61d4, with your R-bs and Rich's.

I'll post a v2 with the last two patches in the series:

this patch (v1 #22) needs an update as you're suggesting, and the change
effects the *commit message* of the next patch (v1 #23) -- that message
includes a diff around nbd_aio_opt_list_meta_context_queries(), and the
*context* in that diff is going to change (by my count) once I modify
this patch.

Thanks!
Laszlo