Re: [Libguestfs] [PATCH libnbd 4/4] lib: Atomically update h->state when leaving the locked region.

On 6/5/19 8:03 AM, Richard W.M. Jones wrote: > On Wed, Jun 05, 2019 at 01:35:32PM +0100, Richard W.M. Jones wrote: >> On Wed, Jun 05, 2019 at 12:15:37PM +0100, Richard W.M. Jones wrote: >>> -#define set_state(h,next_state) ((h)->state) = (next_state) >>> +#define set_next_state(h,_next_state) ((h)->next_state) = (_next_state) >>> +#define get_next_state(h) ((h)->next_state) >>> #define get_state(h) ((h)->state) >> >> So I wonder if it's better to rename get_state as get_last_state or >> get_visible_state? > > Or even get_public_state? get_public_state sounds nice (the state that nbd_connection_state will return). > >> And/or rename get_next_state/set_next_state to >> get_state/set_state? If we rename the public state (which implies that it is frozen while the lock is held), using 'set_state' to adjust the next state is reasonable. >> >> Ideas welcome to make the code clearer. Do we have to use h->next_state everywhere, or can we rely on a stack-allocated variable passed through all the functions? Then again, we're already passing h through to all internal functions, which means h->next_state is already accessible without adding a parameter; but a stack-allocated variable may be harder to pass without lots of churn.