[Spice-devel] [PATCH v2 1/2] UsbDk: Add UsbDk wrapper

Christophe Fergeau cfergeau at redhat.com
Thu Jun 11 08:59:19 PDT 2015


Hey,

I think I forgot to mention this in the initial review, but GModule
might probably have been useful as a wrapper for the GetProcAddress
calls. As we only need to do that on Windows, GModule VS direct win32
API calls do not bring us much.

I'd squash the attached patch in with just some cosmetic changes,
SPICE_DEBUG changed to g_warning in unexpected error cases, and
a licence header more in line with the rest of the code (I picked (C)
2014-2015, let me know if this should be adjusted). One question below:

On Tue, Jun 09, 2015 at 03:53:01PM +0300, Kirill Moizik wrote:
> +void usbdk_api_set_hide_rules(usbdk_api_wrapper *usbdk_api, HANDLE hider_handle, gchar *redirect_on_connect)
> +{
> +    struct usbredirfilter_rule *rules;
> +    int r, count;
> +
> +    r = usbredirfilter_string_to_rules(redirect_on_connect, ",", "|", &rules, &count);
> +    if (r) {
> +        SPICE_DEBUG("auto-connect rules parsing failed with error %d", r);
> +        return;
> +    }
> +
> +    for (int i = 0; i < count; i++) {
> +        USB_DK_HIDE_RULE rule;
> +        rule.Hide  = (uint64_t)rules[i].allow;
> +        rule.Class = (uint64_t)rules[i].device_class;
> +        rule.VID   = (uint64_t)rules[i].vendor_id;
> +        rule.PID   = (uint64_t)rules[i].product_id;
> +        rule.BCD   = (uint64_t)rules[i].device_version_bcd;

Some of the int values stored in the usbredirfilter_rule can be -1, is
usbdk able to cope with these values?

Christophe
-------------- next part --------------
diff --git a/spice-common b/spice-common
--- a/spice-common
+++ b/spice-common
@@ -1 +1 @@
-Subproject commit fec803156b1844a2deb705cf55933294ca60a5ea
+Subproject commit fec803156b1844a2deb705cf55933294ca60a5ea-dirty
diff --git a/src/usbdk_api.c b/src/usbdk_api.c
index f71dee5..f514825 100644
--- a/src/usbdk_api.c
+++ b/src/usbdk_api.c
@@ -1,24 +1,25 @@
 /* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
 /*
-Copyright (C) 2011, 2012 Red Hat, Inc.
+   Copyright (C) 2014-2015 Red Hat, Inc.
 
-* Authors:
-* Dmitry Fleytman <dmitry at daynix.com>
-* Kirill Moizik <kirill at daynix.com>
-his library is free software; you can redistribute it and/or
-This library is free software; you can redistribute it and/or
-modify it under the terms of the GNU Lesser General Public
-License as published by the Free Software Foundation; either
-version 2.1 of the License, or (at your option) any later version.
+   This library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
 
-This library is distributed in the hope that it will be useful,
-but WITHOUT ANY WARRANTY; without even the implied warranty of
-MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-Lesser General Public License for more details.
+   This library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
 
-You should have received a copy of the GNU Lesser General Public
-License along with this library; if not, see <http://www.gnu.org/licenses/>.
+   You should have received a copy of the GNU Lesser General Public
+   License along with this library; if not, see <http://www.gnu.org/licenses/>.
+
+  Authors:
+    Dmitry Fleytman <dmitry at daynix.com>
+    Kirill Moizik <kirill at daynix.com>
 */
+#include <config.h>
 
 #include <windows.h>
 #include <glib-object.h>
@@ -70,20 +71,17 @@ BOOL usbdk_is_driver_installed(void) {
 void usbdk_api_unload(usbdk_api_wrapper *usbdk_api) {
     SPICE_DEBUG("Unloading UsbDk API DLL");
     FreeLibrary(usbdk_api->module);
-    free(usbdk_api);
+    g_free(usbdk_api);
 }
 
 int usbdk_api_load(usbdk_api_wrapper **usbdk_api) {
     SPICE_DEBUG("Loading UsbDk API DLL");
-    *usbdk_api = (usbdk_api_wrapper *) malloc(sizeof(usbdk_api_wrapper));
-    if(*usbdk_api == NULL) {
-        return -1;
-    }
 
+    *usbdk_api = g_new0(usbdk_api_wrapper, 1);
     (*usbdk_api)->module = LoadLibraryA("UsbDkHelper");
     if ((*usbdk_api)->module == NULL) {
-        usbdk_api_unload(*usbdk_api);
         DWORD err = GetLastError();
+        usbdk_api_unload(*usbdk_api);
         SPICE_DEBUG("Failed to load UsbDkHelper.dll, error %lu", err);
         return -1 ;
     }
@@ -91,21 +89,21 @@ int usbdk_api_load(usbdk_api_wrapper **usbdk_api) {
     (*usbdk_api)->CreateHandle = (USBDK_CREATEHIDERHANDLE)
         GetProcAddress((*usbdk_api)->module, "UsbDk_CreateHiderHandle");
     if ((*usbdk_api)->CreateHandle == NULL) {
-        SPICE_DEBUG("CreateHandle");
+        g_warning("Failed to find CreateHandle entry point");
         goto error_unload;
     }
 
     (*usbdk_api)->AddRule = (USBDK_ADDHIDERULE)
         GetProcAddress((*usbdk_api)->module, "UsbDk_AddHideRule");
     if ((*usbdk_api)->AddRule == NULL) {
-        SPICE_DEBUG("AddRule");
+        g_warning("Failed to find AddRule entry point");
         goto error_unload;
     }
 
     (*usbdk_api)->ClearRules = (USBDK_CLEARHIDERULES)
         GetProcAddress((*usbdk_api)->module, "UsbDk_ClearHideRules");
     if ((*usbdk_api)->ClearRules == NULL) {
-        SPICE_DEBUG("ClearRules");
+        g_warning("Failed to find ClearRules entry point");
         goto error_unload;
     }
 
@@ -129,7 +127,7 @@ void usbdk_api_set_hide_rules(usbdk_api_wrapper *usbdk_api, HANDLE hider_handle,
 
     r = usbredirfilter_string_to_rules(redirect_on_connect, ",", "|", &rules, &count);
     if (r) {
-        SPICE_DEBUG("auto-connect rules parsing failed with error %d", r);
+        g_warning("auto-connect rules parsing failed with error %d", r);
         return;
     }
 
@@ -163,4 +161,3 @@ BOOL usbdk_clear_hide_rules(usbdk_api_wrapper *usbdk_api, HANDLE hider_handle) {
 void usbdk_close_hider_handle(usbdk_api_wrapper *usbdk_api, HANDLE hider_handle) {
     return usbdk_api->CloseHiderHandle(hider_handle);
 }
-
diff --git a/src/usbdk_api.h b/src/usbdk_api.h
index e67e720..bc6098d 100644
--- a/src/usbdk_api.h
+++ b/src/usbdk_api.h
@@ -1,23 +1,23 @@
 /* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
 /*
-Copyright (C) 2011, 2012 Red Hat, Inc.
+   Copyright (C) 2014-2015 Red Hat, Inc.
 
-* Authors:
-* Dmitry Fleytman <dmitry at daynix.com>
-* Kirill Moizik <kirill at daynix.com>
+   This library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
 
-This library is free software; you can redistribute it and/or
-modify it under the terms of the GNU Lesser General Public
-License as published by the Free Software Foundation; either
-version 2.1 of the License, or (at your option) any later version.
+   This library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
 
-This library is distributed in the hope that it will be useful,
-but WITHOUT ANY WARRANTY; without even the implied warranty of
-MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-Lesser General Public License for more details.
+   You should have received a copy of the GNU Lesser General Public
+   License along with this library; if not, see <http://www.gnu.org/licenses/>.
 
-You should have received a copy of the GNU Lesser General Public
-License along with this library; if not, see <http://www.gnu.org/licenses/>.
+  Authors:
+    Dmitry Fleytman <dmitry at daynix.com>
+    Kirill Moizik <kirill at daynix.com>
 */
 #ifndef USBDK_HEADER
 #define USBDK_HEADER
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20150611/1aa28199/attachment.sig>


More information about the Spice-devel mailing list