svn commit: r250986 - head/sys/dev/usb

[mdf at FreeBSD.org]

On Sat, May 25, 2013 at 10:09 AM, Hans Petter Selasky
<hselasky at freebsd.org>wrote:

> Author: hselasky
> Date: Sat May 25 17:09:58 2013
> New Revision: 250986
> URL: http://svnweb.freebsd.org/changeset/base/250986
>
> Log:
>   Fix some statical clang analyzer warnings.
>
> Modified:
>   head/sys/dev/usb/usb_device.c
>   head/sys/dev/usb/usb_hub.c
>   head/sys/dev/usb/usb_msctest.c
>
> Modified: head/sys/dev/usb/usb_device.c
>
> ==============================================================================
> --- head/sys/dev/usb/usb_device.c       Sat May 25 16:58:12 2013
>  (r250985)
> +++ head/sys/dev/usb/usb_device.c       Sat May 25 17:09:58 2013
>  (r250986)
> @@ -799,9 +799,6 @@ usb_config_parse(struct usb_device *udev
>                         /* find maximum number of endpoints */
>                         if (ep_max < temp)
>                                 ep_max = temp;
> -
> -                       /* optimalisation */
> -                       id = (struct usb_interface_descriptor *)ed;
>                 }
>         }
>
>
> Modified: head/sys/dev/usb/usb_hub.c
>
> ==============================================================================
> --- head/sys/dev/usb/usb_hub.c  Sat May 25 16:58:12 2013        (r250985)
> +++ head/sys/dev/usb/usb_hub.c  Sat May 25 17:09:58 2013        (r250986)
> @@ -341,7 +341,6 @@ uhub_reattach_port(struct uhub_softc *sc
>
>         DPRINTF("reattaching port %d\n", portno);
>
> -       err = 0;
>         timeout = 0;
>         udev = sc->sc_udev;
>         child = usb_bus_port_get_device(udev->bus,
>
> Modified: head/sys/dev/usb/usb_msctest.c
>
> ==============================================================================
> --- head/sys/dev/usb/usb_msctest.c      Sat May 25 16:58:12 2013
>  (r250985)
> +++ head/sys/dev/usb/usb_msctest.c      Sat May 25 17:09:58 2013
>  (r250986)
> @@ -802,7 +802,6 @@ usb_msc_eject(struct usb_device *udev, u
>         if (sc == NULL)
>                 return (USB_ERR_INVAL);
>
> -       err = 0;
>         switch (method) {
>         case MSC_EJECT_STOPUNIT:
>                 err = bbb_command_start(sc, DIR_IN, 0, NULL, 0,
> @@ -844,6 +843,7 @@ usb_msc_eject(struct usb_device *udev, u
>                 break;
>         default:
>                 DPRINTF("Unknown eject method (%d)\n", method);
> +               err = 0;
>                 break;
>         }
>         DPRINTF("Eject CD command status: %s\n", usbd_errstr(err));
>

I don't know about the top one, but the bottom two are the safer way to
code, and should not have been changed.  Unless we feel guaranteed the
compiler can detect all uninitialized use and will break the build, an
early initialization to a sane value is absolutely the right thing to do,
even if it will be overwritten.  If the compiler feels sure the
initialization isn't needed, it does not have to emit the code.  But any
coding change after the (missing) initialization can create a bug now
(well, it depends on how the code is structured, but from the three lines
of context svn diff provides it's not clear a bug couldn't easily be
introduced).

Thanks,
matthew