On 01/04/2017 03:09 PM, Marc-André Lureau wrote: > Hi > > On Wed, Jan 4, 2017 at 9:26 PM Eric Blake <[email protected]> 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 <[email protected]>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
