[Spice-devel] [PATCH 10/10] Change RedCharDevicePrivate::clients to GList

[Jonathon Jongsma]

On Fri, 2016-09-09 at 17:03 +0200, Christophe Fergeau wrote:
> HHey,
> 
> On Fri, Sep 09, 2016 at 10:47:12AM -0400, Frediano Ziglio wrote:
> > 
> > > 
> > > 
> > > More Ring cleanup
> > 
> > Personally I don't approve the rationale.
> > 
> > Rings are used by Qemu and Linux kernel, Qemu calls us directly and
> > we deal with Linux too. Not counting all BSD code. So surely are
> > not
> > less tested and maintained then glib code.
> 
> In this case, 'Ring' comes from spice-common/common/ring.h, and is
> gplv2/(C) Red Hat, not sure how much of this code is common with what
> is
> used in kernel or BSD land
> 
> 
> > 
> > Rings are not less readable or less type safe then GList or any
> > glib container (personally I think that ring_is_empty is more
> > readable than comparing a container with NULL).
> > Rings are more space and computation efficient.
> > Rings require however to embed container knowledge in the item
> > structures which not desirable in many situations.
> 
> This is my main grip with it wrt to being able to follow the code,
> random 'link' members in structures without it being clear which
> object
> will hold the list. Sometimes there are even several such links for
> different lists, which become even more messy. Or sometimes the link
> is just a 'base' member in the struct (is that inheritance ??). Or we
> have dummy structs just because we need one to hold the link member
> to
> build a list of something.

Not only that, but to get the "contained" item back from the container
element, you have to use the SPICE_CONTAINEROF() macro do a some struct
pointer offset hacks. Every time I use it, I have to scratch my head
and look at the macro definition to figure out which argument should
come first, etc. It's a very awkward system and makes the code *far*
less readable. Maybe the rest of you are so used to this style that
it's intuitive for you, but for me it severely reduces my ability to
comprehend code quickly.

> 
> > 
> > And expanding manually macros is IMHO not more readable nor
> > more maintainable.
> 
> I don't think there is anything preventing from having a macro for
> iteration.
> 
> Christophe