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

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?  And/or rename get_next_state/set_next_state to
get_state/set_state?

Ideas welcome to make the code clearer.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW

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?
> And/or rename get_next_state/set_next_state to
> get_state/set_state?
> 
> Ideas welcome to make the code clearer.
Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW

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.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org