[Spice-devel] [spice-gtk Win32 v3 04/12] Add implementation of SpiceUsbDevice as a gobject (new files spice-usb-device*)
Arnon Gilboa
agilboa at redhat.com
Wed Jun 27 23:45:09 PDT 2012
See notes below.
I guess you defined it only for ref counting, otherwise you would have
used simply a struct (similar to _SpiceUsbDevicePrivate) ?
btw, why define Private and getters and not putting it all public?
Is it the the right gobject-way? no shorter way to do it? (see
spice_msg_in_ref(SpiceMsgIn *in) in spice-channel.c)
Uri Lublin wrote:
> ---
> gtk/spice-usb-device-priv.h | 38 +++++++++++++
> gtk/spice-usb-device.c | 124 +++++++++++++++++++++++++++++++++++++++++++
> gtk/spice-usb-device.h | 57 ++++++++++++++++++++
> 3 files changed, 219 insertions(+), 0 deletions(-)
> create mode 100644 gtk/spice-usb-device-priv.h
> create mode 100644 gtk/spice-usb-device.c
> create mode 100644 gtk/spice-usb-device.h
>
> diff --git a/gtk/spice-usb-device-priv.h b/gtk/spice-usb-device-priv.h
> new file mode 100644
> index 0000000..80b3f5c
> --- /dev/null
> +++ b/gtk/spice-usb-device-priv.h
> @@ -0,0 +1,38 @@
> +/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
> +/*
> + Copyright (C) 2011, 2012 Red Hat, Inc.
> +
> + Red Hat Authors:
> + Uri Lublin <uril at redhat.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 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/>.
> +*/
> +#ifndef __SPICE_USB_DEVICE_PRIV_H__
> +#define __SPICE_USB_DEVICE_PRIV_H__
> +
> +#include <libusb.h>
> +
> +G_BEGIN_DECLS
> +
> +SpiceUsbDevice *spice_usb_device_new(void);
> +void spice_usb_device_set_info(SpiceUsbDevice *self, libusb_device *libdev);
> +
> +guint8 spice_usb_device_get_busnum(SpiceUsbDevice *device);
> +guint8 spice_usb_device_get_devaddr(SpiceUsbDevice *device);
> +guint16 spice_usb_device_get_vid(SpiceUsbDevice *device);
> +guint16 spice_usb_device_get_pid(SpiceUsbDevice *device);
> +
> +G_END_DECLS
> +
> +#endif /* __SPICE_USB_DEVICE_PRIV_H__ */
> diff --git a/gtk/spice-usb-device.c b/gtk/spice-usb-device.c
> new file mode 100644
> index 0000000..24ac1dd
> --- /dev/null
> +++ b/gtk/spice-usb-device.c
> @@ -0,0 +1,124 @@
> +/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
> +/*
> + Copyright (C) 2011, 2012 Red Hat, Inc.
> +
> + Red Hat Authors:
> + Hans de Goede <hdegoede at redhat.com>
> + Uri Lublin <uril at redhat.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 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>
> +#include "spice-usb-device.h"
> +#include "spice-usb-device-priv.h"
> +#include "usbutil.h"
> +#include "glib-compat.h"
> +
> +#define SPICE_USB_DEVICE_GET_PRIVATE(o) \
> + (G_TYPE_INSTANCE_GET_PRIVATE ((o), SPICE_TYPE_USB_DEVICE, SpiceUsbDevicePrivate))
> +
> +struct _SpiceUsbDevicePrivate {
> + guint8 bus;
> + guint8 addr;
> + guint16 vid;
> + guint16 pid;
> +};
> +
> +
> +G_DEFINE_TYPE(SpiceUsbDevice, spice_usb_device, G_TYPE_OBJECT)
> +
> +/* ------------------------------------------------------------------ */
> +
> +static void spice_usb_device_init(SpiceUsbDevice *self)
> +{
> + self->priv = SPICE_USB_DEVICE_GET_PRIVATE(self);
> +}
> +
> +static void spice_usb_device_class_init(SpiceUsbDeviceClass *klass)
> +{
> + g_type_class_add_private(klass, sizeof(SpiceUsbDevicePrivate));
> +}
> +
> +SpiceUsbDevice *spice_usb_device_new(void)
> +{
> + GObject *obj;
> + SpiceUsbDevice *device;
> +
> + obj = g_object_new(SPICE_TYPE_USB_DEVICE, NULL);
> +
>
why obj is good for?
> + device = SPICE_USB_DEVICE(obj);
> +
> + return device;
> +}
> +
> +void spice_usb_device_set_info(SpiceUsbDevice *self, libusb_device *libdev)
> +{
> + SpiceUsbDevicePrivate *priv;
> +
> + g_return_if_fail(SPICE_IS_USB_DEVICE(self));
> + priv = self->priv;
> +
> + g_warn_if_fail(libdev != NULL);
> +
> +#ifdef USE_USBREDIR
>
isn't all of this is used only #ifdef USE_USBREDIR ?
> + if (libdev) {
> + struct libusb_device_descriptor desc;
> + int errcode;
> + const gchar *errstr;
> +
> + priv->bus = libusb_get_bus_number(libdev);
> + priv->addr = libusb_get_device_address(libdev);
> +
> + errcode = libusb_get_device_descriptor(libdev, &desc);
> + if (errcode < 0) {
> + errstr = spice_usbutil_libusb_strerror(errcode);
> + g_warning("cannot get device descriptor for (%p) %d.%d -- %s(%d)",
> + libdev, priv->bus, priv->addr, errstr, errcode);
> + priv->vid = -1;
> + priv->pid = -1;
> + } else {
> + priv->vid = desc.idVendor;
> + priv->pid = desc.idProduct;
> + }
> + }
> +#endif
>
I prefer returning the errorcode.
> +}
> +
> +
>
Do we really need all the
g_return_val_if_fail(SPICE_IS_USB_DEVICE(device), 0) calls when we get
(SpiceUsbDevice *device) ?
I guess we assume here 0 is illegitimate busnum/devaddr/vid/pid. Is it
acceptable?
> +guint8 spice_usb_device_get_busnum(SpiceUsbDevice *device)
> +{
> + g_return_val_if_fail(SPICE_IS_USB_DEVICE(device), 0);
> + return device->priv->bus;
> +}
> +
> +guint8 spice_usb_device_get_devaddr(SpiceUsbDevice *device)
> +{
> + g_return_val_if_fail(SPICE_IS_USB_DEVICE(device), 0);
> + return device->priv->addr;
> +}
> +
> +guint16 spice_usb_device_get_vid(SpiceUsbDevice *device)
> +{
> + g_return_val_if_fail(SPICE_IS_USB_DEVICE(device), 0);
> + return device->priv->vid;
> +}
> +
> +guint16 spice_usb_device_get_pid(SpiceUsbDevice *device)
> +{
> + g_return_val_if_fail(SPICE_IS_USB_DEVICE(device), 0);
> + return device->priv->pid;
> +}
> diff --git a/gtk/spice-usb-device.h b/gtk/spice-usb-device.h
> new file mode 100644
> index 0000000..e740e19
> --- /dev/null
> +++ b/gtk/spice-usb-device.h
> @@ -0,0 +1,57 @@
> +/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
> +/*
> + Copyright (C) 2011, 2012 Red Hat, Inc.
> +
> + Red Hat Authors:
> + Hans de Goede <hdegoede at redhat.com>
> + Uri Lublin <uril at redhat.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 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/>.
> +*/
> +#ifndef __SPICE_USB_DEVICE_H__
> +#define __SPICE_USB_DEVICE_H__
> +
> +G_BEGIN_DECLS
> +
> +#define SPICE_TYPE_USB_DEVICE (spice_usb_device_get_type ())
> +#define SPICE_USB_DEVICE(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), SPICE_TYPE_USB_DEVICE, SpiceUsbDevice))
> +#define SPICE_USB_DEVICE_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST ((klass), SPICE_TYPE_USB_DEVICE, SpiceUsbDeviceClass))
> +#define SPICE_IS_USB_DEVICE(obj) (G_TYPE_CHECK_INSTANCE_TYPE ((obj), SPICE_TYPE_USB_DEVICE))
> +#define SPICE_IS_USB_DEVICE_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), SPICE_TYPE_USB_DEVICE))
> +#define SPICE_USB_DEVICE_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS ((obj), SPICE_TYPE_USB_DEVICE, SpiceUsbDeviceClass))
> +
> +
> +typedef struct _SpiceUsbDevice SpiceUsbDevice;
> +typedef struct _SpiceUsbDeviceClass SpiceUsbDeviceClass;
> +typedef struct _SpiceUsbDevicePrivate SpiceUsbDevicePrivate;
> +
> +struct _SpiceUsbDevice
> +{
> + GObject parent;
> +
> + /*< private >*/
> + SpiceUsbDevicePrivate *priv;
> + /* Do not add fields to this struct */
> +};
> +
> +struct _SpiceUsbDeviceClass
> +{
> + GObjectClass parent_class;
> +};
> +
> +GType spice_usb_device_get_type(void);
> +
> +G_END_DECLS
> +
> +#endif /* __SPICE_USB_DEVICE_H__ */
>
More information about the Spice-devel
mailing list