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(a)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