[Spice-devel] [spice-server v2 02/10] sound: Implement snd_channel_config_socket

[Christophe Fergeau]

On Tue, Jan 24, 2017 at 08:41:00AM -0500, Frediano Ziglio wrote:
> > 
> > This is in preparation for switching SndChannelClient into a proper
> > RedChannelClient. The prototype of the new helper matches what is
> > expected from the RedChannel::config_socket vfunc.
> > 
> > To be able to achieve that, this commit associates the sound channel
> > RedsStream instance with the DummyChannelClient instance we have, and
> > then call snd_channel_config_socket() on that instance.
> > 
> > Based on a patch from Frediano Ziglio <fziglio at redhat.com>
> > 
> > Signed-off-by: Christophe Fergeau <cfergeau at redhat.com>
> > ---
> >  server/dummy-channel-client.c |  2 +
> >  server/dummy-channel-client.h |  1 +
> >  server/sound.c                | 98
> >  +++++++++++++++++++++++--------------------
> >  3 files changed, 55 insertions(+), 46 deletions(-)
> > 
> > diff --git a/server/dummy-channel-client.c b/server/dummy-channel-client.c
> > index 61d5683..4efaa31 100644
> > --- a/server/dummy-channel-client.c
> > +++ b/server/dummy-channel-client.c
> > @@ -1017,24 +977,70 @@ static SndChannelClient *__new_channel(SndChannel
> > *channel, int size, uint32_t c
> >      client->cleanup = cleanup;
> >  
> >      client->channel_client =
> > -        dummy_channel_client_create(RED_CHANNEL(channel), red_client,
> > +        dummy_channel_client_create(RED_CHANNEL(channel), red_client,
> > stream,
> >                                      num_common_caps, common_caps, num_caps,
> >                                      caps);
> >      if (!client->channel_client) {
> >          goto error2;
> >      }
> > +    if
> > (!snd_channel_config_socket(RED_CHANNEL_CLIENT(client->channel_client))) {
> > +        goto error2;
> 
>    g_object_unref(client->channel_client);
>    free(client);
>    return NULL;
> 
> so you don't leak the object and you can't free stream directly as owned by
> client->channel_client.
> Note that code should also unregister the watch but this is not a regression.

The missing g_object_unref(client->channel_client); call is already a problem
before this patch, I'm reluctant to add it in this patch, especially as
this code is going to go away.

> 
> > +    }
> > +
> >      return client;
> >  
> >  error2:
> >      free(client);
> > -
> > -error1:
> >      reds_stream_free(stream);
> >      return NULL;
> >  }

The
free(client);
return(NULL);
you mention above is here

I've removed the reds_stream_free(stream);, thanks for catching this!

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/20170125/1b3403b6/attachment.sig>