Re: [Libguestfs] [libnbd PATCH 0/6] finish wrapping generated C code harder

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.

> > 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 <eblake(a)redhat.com&gt;