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

[Hans Petter Selasky]

On 05/25/13 20:03, mdf at FreeBSD.org wrote:
> 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).

Hi,

The last case is a switch case, and "err" must be set in all cases.

In the second case, "err =" was used two times in a row.

First case is OK. It is leftover code after some earlier patches.

Thanks for the review!

--HPS