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

[Lukas Venhoda]

Hi.



*From: *Victor Toso <lists at victortoso.com>
*Sent: *pátek 8. dubna 2016 9:12
*To: *Lukas Venhoda <lvenhoda at redhat.com>
*Cc: *spice-devel at lists.freedesktop.org
*Subject: *Re: [Spice-devel] [phodav PATCH 3/3 v5] spice-webdavd-windows:
Dismount shared folder on service stop



> @@ -749,6 +749,7 @@ typedef enum _MapDriveEnum
>
>  typedef struct _MapDriveData
>  {
> +  gchar * drive_letter;

I think it is fine to use a gchar instead of gchar* here..



   -I’m using gchar* so that it points to the same gchar as ServiceData.

   -Since it is set in a different thread, I can’t use gchar, and copy the
data back to ServiceData.

>    GCancellable * cancel_map;
>  } MapDriveData;
>
>  static void
> +unmap_drive(const gchar drive_letter)
> +{
> +  gchar local_name[MAX_DRIVE_LETTER_SIZE];
> +  guint32 errn;
> +
> +  g_snprintf(local_name, MAX_DRIVE_LETTER_SIZE, "%c:", drive_letter);
> +  errn = WNetCancelConnection2(local_name, CONNECT_UPDATE_PROFILE, TRUE);
> +
> +  if (errn == NO_ERROR) {
> +    g_debug ("Shared folder unmapped succesfully");
> +  } else if (errn == ERROR_NOT_CONNECTED) {
> +    g_debug ("Drive %c is not connected", drive_letter);
> +  } else {
> +    g_warning ("map_drive error %d", errn);

I'm guessing we can't get a string for the error?



   -Glib doesn’t support this kind of error code

   -Usually with Windows error codes we call GetLastError, which returns a
code,

   -and pass it to g_win32_error_message() to translate i to a string.

   -With WNet code, we already have the error code, and need to pass it to

   -WNetGetLastError, which doesn’t handle strings correctly, and needs

   -a complete wrapper function for allocation, and reallocation of the
error string.

> +  }
> +
> +  return;

No need to return here

> +//Parameter service_data is only used on windows when started as a
service
>  static void
> -run_service (void)
> +run_service (ServiceData * service_data G_GNUC_UNUSED)

ServiceData is defined inside the #ifdef windows but it is being used on
linux
as well here, making the build fail on linux with

<gcc>
    spice/spice-webdavd.c: At top level:
    spice/spice-webdavd.c:926:14: error: unknown type name ‘ServiceData’
     run_service (ServiceData * service_data G_GNUC_UNUSED)
</gcc>

My suggestion is to create all the structures that you need together before
the
functions (top of the file). For the error above, I guess is enough to put
the
drive_letter field with #ifdef... or don't. It'll never be used anyway by
the
linux daemon.

I'll do some tests later on, I think we need to open a few bugs that could
be
closed once that this is integrated with spice-vdagent.

Thanks for your work on this,
  toso

   -My bad. I used a gpointer before, but then I thought, that I will be
using the structure anyway.

   -I can either define the structure outside ifdef, and ifdef the
contents, or fallback to the gpopinter.

   -I think that gpointer would be better suited.



Lukas Venhoda
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20160408/579e28ba/attachment-0001.html>