Re: [PATCH v6 02/12] monitor: Use getter/setter functions for cur_mon

[Kevin Wolf]

Am 05.08.2020 um 09:19 hat Markus Armbruster geschrieben:
> Kevin Wolf <kwolf@redhat.com> writes:
> 
> > Am 04.08.2020 um 14:46 hat Markus Armbruster geschrieben:
> >> > diff --git a/monitor/hmp.c b/monitor/hmp.c
> >> > index d598dd02bb..f609fcf75b 100644
> >> > --- a/monitor/hmp.c
> >> > +++ b/monitor/hmp.c
> >> > @@ -1301,11 +1301,11 @@ cleanup:
> >> >  static void monitor_read(void *opaque, const uint8_t *buf, int size)
> >> >  {
> >> >      MonitorHMP *mon;
> >> > -    Monitor *old_mon = cur_mon;
> >> > +    Monitor *old_mon = monitor_cur();
> >> >      int i;
> >> >  
> >> > -    cur_mon = opaque;
> >> > -    mon = container_of(cur_mon, MonitorHMP, common);
> >> > +    monitor_set_cur(opaque);
> >> > +    mon = container_of(monitor_cur(), MonitorHMP, common);
> >> 
> >> Simpler:
> >> 
> >>        MonitorHMP *mon = container_of(opaque, MonitorHMP, common);
> >
> > opaque is void*, so it doesn't have a field 'common'.
> 
> I actually compile-tested before I sent this.  For once ;)
> 
> Here's container_of():
> 
>     #define container_of(ptr, type, member) ({                      \
>             const typeof(((type *) 0)->member) *__mptr = (ptr);     \
>             (type *) ((char *) __mptr - offsetof(type, member));})
> 
> Its first argument's only use is as an initializer for a pointer
> variable.  Both type * and void * work fine there.

Ah, we just lose type checking.

That's what I get for replying from what I remember from over two months
ago. I was pretty sure I didn't like this way, but went with it because
the other way didn't work. Maybe I just assumed it didn't work, or tried
something different that actually fails. Who knows.

Kevin