Date: Sat, 25 May 2013 20:54:22 +0200 From: Hans Petter Selasky <hps@bitfrost.no> To: mdf@FreeBSD.org Cc: "svn-src-head@freebsd.org" <svn-src-head@freebsd.org>, "svn-src-all@freebsd.org" <svn-src-all@freebsd.org>, "src-committers@freebsd.org" <src-committers@freebsd.org> Subject: Re: svn commit: r250986 - head/sys/dev/usb Message-ID: <51A108DE.7090003@bitfrost.no> In-Reply-To: <CAMBSHm_EgRxbjvTQOu2EDMN%2BMtEJzzbTKSmRKPa9%2B0j3AkqCfQ@mail.gmail.com> References: <201305251709.r4PH9xfv015285@svn.freebsd.org> <CAMBSHm_EgRxbjvTQOu2EDMN%2BMtEJzzbTKSmRKPa9%2B0j3AkqCfQ@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On 05/25/13 20:03, mdf@FreeBSD.org wrote: > On Sat, May 25, 2013 at 10:09 AM, Hans Petter Selasky > <hselasky@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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?51A108DE.7090003>