On 5/10/23 17:25, Eric Blake wrote:
> 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.
Thank you very much for the summary, I didn't have the energy for it :)
Laszlo
>
>>
>> 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 <ebl...@redhat.com>
>
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
