<html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"><meta name="Generator" content="Microsoft Word 15 (filtered medium)"><style></style></head><body lang="CS" link="blue" vlink="#954F72"><div class="WordSection1">Hi <div style="border:none;border-top:solid #e1e1e1 1.0pt;padding:3.0pt 0cm 0cm 0cm">From: <a href="mailto:lists@victortoso.com">Victor Toso</a> Sent: pondělí 2. května 2016 9:08 To: <a href="mailto:lvenhoda@redhat.com">Lukas Venhoda</a> Cc: <a href="mailto:spice-devel@lists.freedesktop.org">spice-devel@lists.freedesktop.org</a> Subject: Re: [Spice-devel] [phodav PATCH 3/3 v5] spice-webdavd-windows: Dismount shared folder on service stop</div> 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 </div></body></html>