On Fri, Aug 12, 2011 at 04:50:33PM +0200, Hans de Goede wrote: > diff --git a/gtk/usb-device-manager.c b/gtk/usb-device-manager.c > new file mode 100644 > index 0000000..2287383 > --- /dev/null > +++ b/gtk/usb-device-manager.c > @@ -0,0 +1,459 @@ > +/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */ > +/* > + Copyright (C) 2011 Red Hat, Inc. > + > + Red Hat Authors: > + Hans de Goede <[email protected]> > + > + 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. > + > + 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/>. > +*/ > + > +#include "config.h" > + > +#include <glib-object.h> > + > +#ifdef USE_USBREDIR > +#include <gusb/gusb-source.h> > +#include <gusb/gusb-device-list.h> > +#include "channel-usbredir-priv.h" > +#endif > + > +#include "spice-client.h" > +#include "spice-marshal.h" > + > +#define SPICE_USB_DEVICE_MANAGER_GET_PRIVATE(obj) > \ > + (G_TYPE_INSTANCE_GET_PRIVATE ((obj), SPICE_TYPE_USB_DEVICE_MANAGER, > SpiceUsbDeviceManagerPrivate)) > + > +enum { > + PROP_0, > + PROP_MAIN_CONTEXT, > + PROP_AUTO_CONNECT, > +}; > + > +enum > +{ > + DEVICE_ADDED_SIGNAL, > + DEVICE_REMOVED_SIGNAL, > + LAST_SIGNAL, > +}; > + > +struct _SpiceUsbDeviceManagerPrivate { > + GMainContext *main_context; > + gboolean auto_connect; > + GError *usb_init_error; > +#ifdef USE_USBREDIR > + GUsbContext *context; > + GUsbDeviceList *devlist; > + GUsbSource *source; > +#endif > + GPtrArray *devices; > + GPtrArray *channels; > +}; > + > +#ifdef USE_USBREDIR > +static void spice_usb_device_manager_dev_added(GUsbDeviceList *devlist, > + GUsbDevice *device, > + GUdevDevice *udev, > + gpointer user_data); > +static void spice_usb_device_manager_dev_removed(GUsbDeviceList *devlist, > + GUsbDevice *device, > + GUdevDevice *udev, > + gpointer user_data); > +#endif > + > +static guint signals[LAST_SIGNAL] = { 0, }; > + > +G_DEFINE_TYPE(SpiceUsbDeviceManager, spice_usb_device_manager, G_TYPE_OBJECT) > +G_DEFINE_BOXED_TYPE(SpiceUsbDevice, spice_usb_device, g_object_ref, > g_object_unref)
Why do you introduce this SpiceUsbDevice opaque type? You won't want to
keep the fact that it's a GUsbDevice as an implementation detail so that it
can be changed to an object inheriting from GUsbDevice later?
> +
> +static void spice_usb_device_manager_init(SpiceUsbDeviceManager *manager)
> +{
> + SpiceUsbDeviceManagerPrivate *priv;
> +
> + priv = SPICE_USB_DEVICE_MANAGER_GET_PRIVATE(manager);
> + manager->priv = priv;
> +
> + priv->main_context = NULL;
> + priv->channels = g_ptr_array_new();
> + priv->devices = g_ptr_array_new_with_free_func((GDestroyNotify)
> + g_object_unref);
I think I'd have used a GHashTable for the list of devices since
g_ptr_array_remove will have to iterate over the array to find the device
to remove. It could even store the device => channel association, with the
GPtrArray of channels which could become a GList * of non-associated
channels.
However, it's more a matter of personal coding style, your way of doing
this is fine with me too.
> +#ifdef USE_USBREDIR
> + priv->source = NULL;
> + priv->devlist = NULL;
> + priv->usb_init_error = NULL;
For what it's worth, the various fields will be set to NULL by glib when
the object is created.
The rest looked good to me,
Thanks,
Christophe
pgpQ0btVMhzL8.pgp
Description: PGP signature
_______________________________________________ Spice-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/spice-devel
