[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: *pondělí 2. května 2016 9:08
*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



Hey,

Sorry, I forgot to reply to this sooner.

On Fri, Apr 08, 2016 at 01:10:43PM +0200, Lukas Venhoda wrote:
> >
> > > @@ -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.

Why not use the same struct in both contexts and a mutex to avoid
concurrency
problems?

Cheers,
  toso

            Because ServiceData is not used, when called with --no-service.

            It won’t be created on stack, and would need to be allocated
afterwards in run_service, just so that we don’t use gchar*.


>
> >    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/20160502/270b854c/attachment.html>