[Spice-devel] [PATCH 3/4] sound: Remove dead code in client constructors

[Jonathon Jongsma]

On Fri, 2017-04-21 at 05:10 -0400, Frediano Ziglio wrote:
> > 
> > When a new PlaybackChannelClient or RecordChannelClient is created,
> > there are several places where we make decisions based on whether
> > the
> > client is active or not. But these checks are done before the
> > 'active'
> > flag is ever set, so this code is effectively dead. This has been
> > the
> > case since the very first commit in this repository.
> > 
> > Signed-off-by: Jonathon Jongsma <jjongsma at redhat.com>
> 
> Looking at the entire history looks like it's not exact.
> 
> It seems the problem was introduced by
> 
> commit 6fdcb9310d8bd0383af97951ff7ba270c5ba08be
> Author: Gerd Hoffmann <kraxel at redhat.com>
> Date:   Tue Apr 13 14:35:32 2010 +0200
> 
>     SoundInterfaces: redesign

Oh, you're right. I totally missed the fact that the old interface-
> plug() function had an output argument that modified 'active'. I'll

modify the commit log.

> 
> and attempted to be fixed by
> 
> commit d351bb35ce981d39e839528f4a23bc5b501adab8
> Author: Gerd Hoffmann <kraxel at redhat.com>
> Date:   Thu May 20 13:32:00 2010 +0200
> 
>     sound channels: restart audio on client reconnect.
> 
> But since then that code was dead code.
> Looks like any possible regression introduced by the
> first patch was fixed (I hope so after 7 years).

Right, that change does seem to cover potential regressions. And after
7 years we can probably assume we're OK here. 

> 
> Frediano
> 
> > ---
> >  server/sound.c | 10 ----------
> >  1 file changed, 10 deletions(-)
> > 
> > diff --git a/server/sound.c b/server/sound.c
> > index fb8f5c6..7490a19 100644
> > --- a/server/sound.c
> > +++ b/server/sound.c
> > @@ -1039,7 +1039,6 @@ playback_channel_client_constructed(GObject
> > *object)
> >      RedChannel *red_channel = red_channel_client_get_channel(rcc);
> >      SndChannel *channel = SND_CHANNEL(red_channel);
> >      RedClient *red_client = red_channel_client_get_client(rcc);
> > -    RedsState *reds = red_channel_get_server(red_channel);
> >      SndChannelClient *scc = SND_CHANNEL_CLIENT(playback_client);
> >  
> >      G_OBJECT_CLASS(playback_channel_client_parent_class)-
> > > constructed(object);
> > 
> > @@ -1071,15 +1070,9 @@ playback_channel_client_constructed(GObject
> > *object)
> >          return;
> >      }
> >      snd_set_command(scc, SND_PLAYBACK_MODE_MASK);
> > -    if (scc->active) {
> > -        snd_set_command(scc, SND_CTRL_MASK);
> > -    }
> >      if (channel->volume.volume_nchannels) {
> >          snd_set_command(scc, SND_VOLUME_MUTE_MASK);
> >      }
> > -    if (scc->active) {
> > -        reds_disable_mm_time(reds);
> > -    }
> >  
> >      if (channel->active) {
> >          snd_playback_start(channel);
> > @@ -1272,9 +1265,6 @@ record_channel_client_constructed(GObject
> > *object)
> >      if (channel->volume.volume_nchannels) {
> >          snd_set_command(scc, SND_VOLUME_MUTE_MASK);
> >      }
> > -    if (scc->active) {
> > -        snd_set_command(scc, SND_CTRL_MASK);
> > -    }
> >  
> >      if (channel->active) {
> >          snd_record_start(channel);