[Spice-devel] [vdagent-win PATCH 08/13] file_xfer: Use destructor for FileXferTask

[Christophe Fergeau]

On Mon, May 28, 2018 at 09:58:01AM +0100, Frediano Ziglio wrote:
> Limit too much manual work.
> By default delete the file, unless success() is called.
> 
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
>  vdagent/file_xfer.cpp | 23 ++++++++++++++---------
>  vdagent/file_xfer.h   |  4 +++-
>  2 files changed, 17 insertions(+), 10 deletions(-)
> 
> diff --git a/vdagent/file_xfer.cpp b/vdagent/file_xfer.cpp
> index 8203b99..e96065b 100644
> --- a/vdagent/file_xfer.cpp
> +++ b/vdagent/file_xfer.cpp
> @@ -46,7 +46,6 @@ void FileXfer::reset()
>  
>      for (iter = _tasks.begin(); iter != _tasks.end(); iter++) {
>          task = iter->second;
> -        task->cancel();
>          delete task;
>      }
>      _tasks.clear();
> @@ -181,14 +180,11 @@ bool FileXfer::handle_data(VDAgentFileXferDataMessage* data,
>          return false;
>      }
>      vd_printf("%u completed", iter->first);
> +    task->success();
>      status->result = VD_AGENT_FILE_XFER_STATUS_SUCCESS;
>  
>  fin:
>      if (task) {
> -        CloseHandle(task->handle);
> -        if (status->result != VD_AGENT_FILE_XFER_STATUS_SUCCESS) {
> -            DeleteFile(task->name);
> -        }
>          _tasks.erase(iter);
>          delete task;
>      }
> @@ -196,10 +192,20 @@ fin:
>      return true;
>  }
>  
> -void FileXferTask::cancel()
> +FileXferTask::~FileXferTask()
> +{
> +    if (handle != INVALID_HANDLE_VALUE) {
> +        CloseHandle(handle);
> +        DeleteFile(name);
> +    }
> +}
> +
> +void FileXferTask::success()
>  {
> -    CloseHandle(handle);
> -    DeleteFile(name);
> +    if (handle != INVALID_HANDLE_VALUE) {
> +        CloseHandle(handle);
> +        handle = INVALID_HANDLE_VALUE;
> +    }

Maybe add a comment explaining this is to prevent the destructor from
calling DeleteFile()?

Looks good otherwise.

>  }
>  
>  void FileXfer::handle_status(VDAgentFileXferStatusMessage* status)
> @@ -218,7 +224,6 @@ void FileXfer::handle_status(VDAgentFileXferStatusMessage* status)
>          return;
>      }
>      task = iter->second;
> -    task->cancel();
>      _tasks.erase(iter);
>      delete task;
>  }
> diff --git a/vdagent/file_xfer.h b/vdagent/file_xfer.h
> index 747c29c..41f677a 100644
> --- a/vdagent/file_xfer.h
> +++ b/vdagent/file_xfer.h
> @@ -30,12 +30,14 @@ struct FileXferTask {
>          lstrcpyn(name, _name, ARRAYSIZE(name));
>          name[ARRAYSIZE(name)-1] = 0;
>      }
> +    ~FileXferTask();
> +
>      HANDLE handle;
>      uint64_t size;
>      uint64_t pos;
>      TCHAR name[MAX_PATH];
>  
> -    void cancel();
> +    void success();
>  };
>  
>  typedef std::map<uint32_t, FileXferTask*> FileXferTasks;
> -- 
> 2.17.0
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20180528/2790168d/attachment-0001.sig>