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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi.

 

From: Victor Toso
Sent: pátek 8. dubna 2016 9:12
To: Lukas Venhoda
Cc: spice-devel@xxxxxxxxxxxxxxxxxxxxx
Subject: Re: [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

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]