> > 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@xxxxxxxxxx> > > > > --- > > > > 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 > I missed tha main channel so now the patch make stuff even better, https://cgit.freedesktop.org/~fziglio/spice-server/commit/?h=common_graphics_channel_client&id=9ae24c5bf2528e6debf3465ec8760983007ea108 About possible improves I'm quite of the opinion that would be better to have a follow up. Changing RedChannelCapabilities would require lot of additional changes here and there and I would then prefer to have also a single property. Arriving till the end to have to split the structure for 2 properties doesn't seems really good. Looking at all paths common+channel caps arrive from client in the link message (the first raw message from the client) and are just forwarded together (previously as 4 parameters), then converted to 2 GArrays then converted back to common+channel caps. Looking at all path would make more sense to have always together. I would box the RedChannelCapabilities. Unfortunately 3/3 depends on this. I'll update this and add a follow up with the boxing of current structure. Frediano _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel