[4/4] usb: use DPRINTF instead of printf for some simple cases

Signed-off-by: Brad Hards <bradh@frogmouth.net>
---
 usb-linux.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

On Wed, Apr 13, 2011 at 10:45 AM, Brad Hards <bradh@frogmouth.net> wrote:
> Signed-off-by: Brad Hards <bradh@frogmouth.net>
> ---
>  usb-linux.c |   16 ++++++++--------
>  1 files changed, 8 insertions(+), 8 deletions(-)

Some of these printfs look like they might be useful given that the
current USB support is known to be imperfect and frequently causes
questions from users.  By changing them to DPRINTF() you are making
these message available only to developers and not users.

Any thoughts from people who use or have written the usb-linux.c code?

Stefan

Hi,

On 04/13/2011 11:45 AM, Brad Hards wrote:
> Signed-off-by: Brad Hards<bradh@frogmouth.net>
> ---
>   usb-linux.c |   16 ++++++++--------
>   1 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/usb-linux.c b/usb-linux.c
> index 1f33c2c..b02a0f9 100644
> --- a/usb-linux.c
> +++ b/usb-linux.c
> @@ -233,8 +233,8 @@ static void async_complete(void *opaque)
>                   return;
>               }
>               if (errno == ENODEV&&  !s->closing) {
> -                printf("husb: device %d.%d disconnected\n",
> -                       s->bus_num, s->addr);
> +                DPRINTF("husb: device %d.%d disconnected\n",
> +                        s->bus_num, s->addr);
>                   usb_host_close(s);
>                   usb_host_auto_check(NULL);
>                   return;

I think this one should stay a regular printf, in case the disconnect is
unintentional people may think it is a qemu problem without the printf.

> @@ -320,7 +320,7 @@ static int usb_host_claim_interfaces(USBHostDevice *dev, int configuration)
>           }
>           config_descr_len = dev->descr[i];
>
> -        printf("husb: config #%d need %d\n", dev->descr[i + 5], configuration);
> +        DPRINTF("husb: config #%d need %d\n", dev->descr[i + 5], configuration);
>
>           if (configuration<  0 || configuration == dev->descr[i + 5]) {
>               configuration = dev->descr[i + 5];

Ack.

> @@ -359,7 +359,7 @@ static int usb_host_claim_interfaces(USBHostDevice *dev, int configuration)
>           ret = ioctl(dev->fd, USBDEVFS_CLAIMINTERFACE,&interface);
>           if (ret<  0) {
>               if (errno == EBUSY) {
> -                printf("husb: update iface. device already grabbed\n");
> +                DPRINTF("husb: update iface. device already grabbed\n");
>               } else {
>                   perror("husb: failed to claim interface");
>               }

Nack, this is an error condition, so it should not be a DPRINTF.

> @@ -368,8 +368,8 @@ static int usb_host_claim_interfaces(USBHostDevice *dev, int configuration)
>           }
>       }
>
> -    printf("husb: %d interfaces claimed for configuration %d\n",
> -           nb_interfaces, configuration);
> +    DPRINTF("husb: %d interfaces claimed for configuration %d\n",
> +            nb_interfaces, configuration);
>
>       dev->ninterfaces   = nb_interfaces;
>       dev->configuration = configuration;

Ack.

> @@ -929,7 +929,7 @@ static int usb_host_open(USBHostDevice *dev, int bus_num,
>       if (dev->fd != -1) {
>           goto fail;
>       }
> -    printf("husb: open device %d.%d\n", bus_num, addr);
> +    DPRINTF("husb: open device %d.%d\n", bus_num, addr);
>
>       if (!usb_host_device_path) {
>           perror("husb: USB Host Device Path not set");

Ack.

> @@ -984,7 +984,7 @@ static int usb_host_open(USBHostDevice *dev, int bus_num,
>           goto fail;
>       }
>
> -    printf("husb: grabbed usb device %d.%d\n", bus_num, addr);
> +    DPRINTF("husb: grabbed usb device %d.%d\n", bus_num, addr);
>
>       ret = usb_linux_update_endp_table(dev);
>       if (ret) {

Ack.

On Wed, 13 Apr 2011 10:52:37 pm Hans de Goede wrote:
> > @@ -359,7 +359,7 @@ static int usb_host_claim_interfaces(USBHostDevice
> > *dev, int configuration)
> >
> >           ret = ioctl(dev->fd, USBDEVFS_CLAIMINTERFACE,&interface);
> >           if (ret<  0) {
> >               if (errno == EBUSY) {
> >
> > -                printf("husb: update iface. device already grabbed\n");
> > +                DPRINTF("husb: update iface. device already grabbed\n");
> >
> >               } else {
> >                   perror("husb: failed to claim interface");
> >               }
> 
> Nack, this is an error condition, so it should not be a DPRINTF.
Then should it go to stderr instead of stdout?

(There are other places in this code where we use fprintf(stderr, ...) to 
indicate error conditions.)

Brad

On Thursday 14 April 2011 08:01:43 Brad Hards wrote:
> On Wed, 13 Apr 2011 10:52:37 pm Hans de Goede wrote:
> > > @@ -359,7 +359,7 @@ static int usb_host_claim_interfaces(USBHostDevice
> > > *dev, int configuration)
> > > 
> > >           ret = ioctl(dev->fd, USBDEVFS_CLAIMINTERFACE,&interface);
> > >           if (ret<  0) {
> > >           
> > >               if (errno == EBUSY) {
> > > 
> > > -                printf("husb: update iface. device already
> > > grabbed\n"); +                DPRINTF("husb: update iface. device
> > > already grabbed\n");
> > > 
> > >               } else {
> > >               
> > >                   perror("husb: failed to claim interface");
> > >               
> > >               }
> > 
> > Nack, this is an error condition, so it should not be a DPRINTF.
> 
> Then should it go to stderr instead of stdout?
> 
> (There are other places in this code where we use fprintf(stderr, ...) to
> indicate error conditions.)
ping?

Brad