[Spice-devel] [PATCH spice-server 1a/3] red-channel: Use directly a GArray to pass capabilities

[Christophe Fergeau]

On Tue, Feb 28, 2017 at 12:14:25PM -0500, Frediano Ziglio wrote:
> > 
> > On Tue, Feb 28, 2017 at 09:43:33AM +0000, Frediano Ziglio wrote:
> > > Capabilities where almost always passed using 2 arguments,
> > > a number of elements and an array but then before using
> > > these were converted to a GArray.
> > > Converting to GArray much earlier allows to easily pass
> > > the capabilities around.
> > 
> > Hey, nice improvement!
> > 
> > > 
> > > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > > ---
> > > I don't know if is worth defining an abstract type
> > > instead of using a simple GArray.
> > > Maybe a "typedef GArray RedCapabilities" could help future
> > > replacement of the GArray?
> > 
> > There's already a RedChannelCapabilities in red-channel.h, I'd reuse
> > this one.
> > 
> 
> Mumble... contains both common_caps and normal ones but can
> be done, we always pass them together.
> I was thinking about just joining caps+num.

Yeah, I know, but during review I came across that preexisting type.
Since that type is already there, imo it's even cleaner to pass a single
set of caps rather than always passing the 2 arrays.

> 
> > > On the other side this patch moves most of the array boxing
> > > into a single function.
> > 
> > I'd create the GArray one level higher, in reds_channel_do_link() rather
> > than red_channel_connect, let reds.c do the grunt work, and have
> > RedChannel deal with higher level types.
> > 
> > Christophe
> > 
> 
> If you want to reuse RedChannelCapabilities we could just box this
> type and avoid the GArray change.
> Or use GArray in a follow up (or not!).
> Other work for reds.c? Is not enough the cacophonous stuff in here?

I think I'd change RedChannelCapabilities to store 2 GArrays and pass
that around. As RedChannel properties, it's probably easier to keep 2
different GArrays (?)

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170228/65e33869/attachment-0001.sig>