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 <ebl...@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
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
