On 01/04/2017 03:09 PM, Marc-André Lureau wrote: > Hi > > On Wed, Jan 4, 2017 at 9:26 PM Eric Blake wrote: > >> On 12/12/2016 04:42 PM, Marc-André Lureau wrote: >>> Use a single allocation for CharDriverState, this avoids extra >>> allocations & pointers, and is a step towards more object-oriented >>> CharDriver. >>> >>> Gtk console is a bit peculiar, gd_vc_chr_set_echo >> >> Truncated paragraph? >> > > yes, fixed > >>> @@ -270,7 +270,7 @@ static int baum_deferred_init(BaumDriverState *baum) >>> /* The serial port can receive more of our data */ >>> static void baum_accept_input(struct CharDriverState *chr) >>> { >>> - BaumDriverState *baum = chr->opaque; >>> + BaumDriverState *baum = (BaumDriverState *)chr; >> >> It might be a little nicer to use: >> >> BaumDriverState *baum = container_of(chr, BaumDriverState, parent); >> >> > The follow-up of the series converts it to the more appropriate > BaumChardev *baum = BAUM_CHARDEV(obj); > > So considering that this is temporary commit, do I have to change that? Ah, nice. So BAUM_CHARDEV(obj) will be a nice wrapper around container_of(). I can live with it being temporary (although a note in the commit message can't hurt). > > to avoid a cast and work even if the members are rearranged within the >> structure. You can even write a one-line helper function to hide the >> cast behind a more legible semantic (for example, see to_qov() in >> qobject-output-visitor.c). My example of to_qov() is matched by your BAUM_CHARDEV() macro. >> My biggest gripe is the number of casts that could be container_of() >> instead; but otherwise it looks like a sane conversion. >> >> > thanks, the goal is to get rid of them, but not in this commit :) See > "chardev: qom-ify". That's what I get for only reviewing the series one patch at a time. I'm fine with temporary and partial hacks, but calling them out as such makes review easier if we know it's going to improve later. Since I think you've answered my questions, then with an improved commit message, v2 can add: Reviewed-by: Eric Blake -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org