[Spice-devel] [PATCH 3/4] vdagent: move file xfer finalize to a function

[Pavel Grunt]

On Fri, 2017-05-12 at 14:19 +0200, Victor Toso wrote:
> Hi,
> 
> On Fri, May 12, 2017 at 02:11:12PM +0200, Pavel Grunt wrote:
> > On Fri, 2017-05-12 at 13:19 +0200, Victor Toso wrote:
> > > From: Victor Toso <me at victortoso.com>
> > > 
> > > This patch creates vdagent_stop_file_xfer() to finalize and stop
> > > file xfer. Moving this code to a function removes some
> > > duplication.
> > > 
> > > Signed-off-by: Victor Toso <victortoso at redhat.com>
> > > ---
> > >  src/vdagent/vdagent.c | 26 +++++++++++++++-----------
> > >  1 file changed, 15 insertions(+), 11 deletions(-)
> > > 
> > > diff --git a/src/vdagent/vdagent.c b/src/vdagent/vdagent.c
> > > index 08ebd92..4f8079f 100644
> > > --- a/src/vdagent/vdagent.c
> > > +++ b/src/vdagent/vdagent.c
> > > @@ -109,6 +109,17 @@ vdagent_start_file_xfer(void)
> > >      return (vdagent_file_xfers != NULL);
> > >  }
> > >  
> > > +static bool
> > > +vdagent_stop_file_xfer(void)
> > 
> > maybe clear instead of stop?
> 
> clear looks okay here
> 
> > > +{
> > > +    if (vdagent_file_xfers == NULL)
> > > +        return false;
> > > +
> > > +    vdagent_file_xfers_destroy(vdagent_file_xfers);
> > > +    vdagent_file_xfers = NULL;
> > > +    return true;
> > > +}
> > > +
> > >  static void daemon_read_complete(struct udscs_connection
> > > **connp,
> > >      struct udscs_message_header *header, uint8_t *data)
> > >  {
> > > @@ -160,10 +171,7 @@ static void daemon_read_complete(struct
> > > udscs_connection **connp,
> > >          if (debug)
> > >              syslog(LOG_DEBUG, "Disabling file-xfers");
> > >  
> > > -        if (vdagent_file_xfers != NULL) {
> > > -            vdagent_file_xfers_destroy(vdagent_file_xfers);
> > > -            vdagent_file_xfers = NULL;
> > > -        }
> > > +        vdagent_stop_file_xfer();
> > >          break;
> > >      case VDAGENTD_AUDIO_VOLUME_SYNC: {
> > >          VDAgentAudioVolumeSync *avs = (VDAgentAudioVolumeSync
> > > *)data;
> > > @@ -185,10 +193,8 @@ static void daemon_read_complete(struct
> > > udscs_connection **connp,
> > >          break;
> > >      case VDAGENTD_CLIENT_DISCONNECTED:
> > >          vdagent_x11_client_disconnected(x11);
> > > -        if (vdagent_file_xfers != NULL) {
> > > -            vdagent_file_xfers_destroy(vdagent_file_xfers);
> > > -            vdagent_file_xfers =
> > > vdagent_file_xfers_create(client,
> > > fx_dir,
> > > -                                                           fx_o
> > > pen_
> > > dir, debug);
> > > +        if (vdagent_stop_file_xfer() &&
> > > !vdagent_start_file_xfer())
> > > {
> > > +            syslog(LOG_WARNING, "File transfer is disabled");
> > 
> > the warning is new and will be repeated
> 
> How?
it is not in the current code. the warning will be repeated every time
client disconnects.
> 
> > 
> > ot: all that is needed is to clear vdagent_file_xfers->xfers
> 
> Why? On CLIENT_DISCONNECT we were restarting it.
yes, but in fact is only needed to clear the hashtable - other values
of the struct remain the same, no? It is "off topic"/unrelated to your
patch.

> 
> > 
> > >          }
> > >          break;
> > >      default:
> > > @@ -400,9 +406,7 @@ reconnect:
> > >          udscs_client_handle_fds(&client, &readfds, &writefds);
> > >      }
> > >  
> > > -    if (vdagent_file_xfers != NULL) {
> > > -        vdagent_file_xfers_destroy(vdagent_file_xfers);
> > > -    }
> > > +    vdagent_stop_file_xfer();
> > >      vdagent_x11_destroy(x11, client == NULL);
> > >      udscs_destroy_connection(&client);
> > >      if (!quit && do_daemonize)