[Spice-devel] [phodav PATCH 3/3 v3] spice-webdavd-windows: Dismount shared folder on service stop

[Victor Toso]

On Thu, Mar 24, 2016 at 04:39:42PM +0100, Lukas Venhoda wrote:
> When stopping the service, automatically disconnect shared folder on
> windows. Not dismounting could lead to multiple shared folders.
> ---
> Changes since v3:
>  - Better handeling of string names
>  - Syntax cleanup
>  - Remove global variable drive_letter
>     - Now scans for mapped drive and unmaps it
> 
> Changes since v2:
>  - None
> 
> Changes since v1:
>  - New patch
> ---
>  spice/spice-webdavd.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/spice/spice-webdavd.c b/spice/spice-webdavd.c
> index be2bf90..93b6178 100644
> --- a/spice/spice-webdavd.c
> +++ b/spice/spice-webdavd.c
> @@ -842,6 +842,27 @@ map_drive(const gchar drive_letter)
>  }
> 
>  static void
> +unmap_drive(const gchar drive_letter)
> +{
> +  #define LOCAL_NAME_SIZE 3

Same comment from previous patch

> +  gchar local_name[LOCAL_NAME_SIZE];
> +  guint32 errn;
> +
> +  g_snprintf(local_name, LOCAL_NAME_SIZE, "%c:", drive_letter);
> +  errn = WNetCancelConnection2(local_name, CONNECT_UPDATE_PROFILE, TRUE);
> +
> +  if (errn == NO_ERROR) {
> +    g_debug ("unmap_drive ok");
> +  } else if (errn == ERROR_NOT_CONNECTED) {
> +    g_debug ("drive not connected");
> +  } else {
> +    g_warning ("map_drive error %d", errn);

I think you could use WNetGetLastError

> +  }
> +
> +  return;

you don't need this return

> +}
> +
> +static void
>  map_drive_cb(GTask *task,
>               gpointer source_object,
>               gpointer task_data,
> @@ -972,6 +993,7 @@ service_ctrl_handler (DWORD ctrl, DWORD type, LPVOID data, LPVOID ctx)
>      {
>      case SERVICE_CONTROL_STOP:
>      case SERVICE_CONTROL_SHUTDOWN:
> +        unmap_drive (get_spice_folder_letter ());

I don't like this :)
In order to unmap_drive you are going to iterate again to check
letters and so on, just to unmap something that you should know what it
is at this point.

I would consider using a struct operation that you can create on
service_main and pass on RegisterServiceCtrlHandlerEx()... this struct
should hold at least the drive that is mapped so you can use it on
unmap_drive.

Let me know what you think,
  toso

>          quit (SIGTERM);
>          service_status.dwCurrentState = SERVICE_STOP_PENDING;
>          SetServiceStatus (service_status_handle, &service_status);
> --
> 2.5.5
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel