Re: [Qemu-devel] [PATCH] Make usb-bt-dongle configurable

[Andreas Färber]

Am 19.08.2013 12:48, schrieb Miroslav Rezanina:
> usb-bt-dongle device can't be disabled as there's dependency in vl.c file. 
> This patch add preprocesor condition to be able to disable it.

Please limit to 76 chars per line (check `git log` output).

> 
> Signed-off-by: Miroslav Rezanina <address@hidden>
> ---
>  hw/usb/Makefile.objs |  1 -
>  vl.c                 | 18 ++++++++++++++----
>  2 files changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/usb/Makefile.objs b/hw/usb/Makefile.objs
> index f9695e7..8892ffd 100644
> --- a/hw/usb/Makefile.objs
> +++ b/hw/usb/Makefile.objs
> @@ -20,7 +20,6 @@ common-obj-$(CONFIG_USB_SERIAL)       += dev-serial.o
>  common-obj-$(CONFIG_USB_NETWORK)      += dev-network.o
>  
>  # FIXME: make configurable too
> -CONFIG_USB_BLUETOOTH := y

You probably should delete the FIXME alongside?

>  common-obj-$(CONFIG_USB_BLUETOOTH)    += dev-bluetooth.o
>  
>  ifeq ($(CONFIG_USB_SMARTCARD),y)
> diff --git a/vl.c b/vl.c
> index f422a1c..4330b6d 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1526,8 +1526,10 @@ static void configure_msg(QemuOpts *opts)
>  
>  static int usb_device_add(const char *devname)
>  {
> -    const char *p;
>      USBDevice *dev = NULL;
> +#if  defined(CONFIG_USB_BLUETOOTH) || !defined(CONFIG_LINUX)

Double space.

> +    const char *p;
> +#endif
>  
>      if (!usb_enabled(false)) {
>          return -1;
> @@ -1543,15 +1545,23 @@ static int usb_device_add(const char *devname)
>      /* only the linux version is qdev-ified, usb-bsd still needs this */
>      if (strstart(devname, "host:", &p)) {
>          dev = usb_host_device_open(usb_bus_find(-1), p);
> -    } else
> +        goto devtest;
> +    } 
>  #endif
> +#ifdef CONFIG_USB_BLUETOOTH
>      if (!strcmp(devname, "bt") || strstart(devname, "bt:", &p)) {
>          dev = usb_bt_init(usb_bus_find(-1),
>                            devname[2] ? hci_init(p)
>                                       : bt_new_hci(qemu_find_bt_vlan(0)));
> -    } else {
> -        return -1;
> +        goto devtest;
>      }
> +#endif
> +
> +    return -1;
> +
> +#if  defined(CONFIG_USB_BLUETOOTH) || !defined(CONFIG_LINUX)
> +devtest:
> +#endif

Why put only the label in an #if block? The below is dead code if not
reached via devtest.

>      if (!dev)
>          return -1;
>  

Let's also not forget to CC Gerd as USB maintainer.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg