From owner-svn-src-all@FreeBSD.ORG Sat May 25 18:53:15 2013 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 0054C1A7; Sat, 25 May 2013 18:53:14 +0000 (UTC) (envelope-from hps@bitfrost.no) Received: from mta.bitpro.no (mta.bitpro.no [92.42.64.202]) by mx1.freebsd.org (Postfix) with ESMTP id 8C321F97; Sat, 25 May 2013 18:53:14 +0000 (UTC) Received: from mail.bitfrost.no (mail.bitfrost.no [46.29.221.36]) by mta.bitpro.no (Postfix) with ESMTP id 0963A7A11D; Sat, 25 May 2013 20:53:07 +0200 (CEST) X-Virus-Scanned: Debian amavisd-new at bitfrost.no Received: from laptop015.hselasky.homeunix.org (cm-176.74.213.204.customer.telag.net [176.74.213.204]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) (Authenticated sender: hanspetter) by mail.bitfrost.no (Postfix) with ESMTPSA id 9FCAC20551; Sat, 25 May 2013 20:52:52 +0200 (CEST) Message-ID: <51A108DE.7090003@bitfrost.no> Date: Sat, 25 May 2013 20:54:22 +0200 From: Hans Petter Selasky Organization: Bitfrost A/S MIME-Version: 1.0 To: mdf@FreeBSD.org Subject: Re: svn commit: r250986 - head/sys/dev/usb References: <201305251709.r4PH9xfv015285@svn.freebsd.org> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: "svn-src-head@freebsd.org" , "svn-src-all@freebsd.org" , "src-committers@freebsd.org" X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 25 May 2013 18:53:15 -0000 On 05/25/13 20:03, mdf@FreeBSD.org wrote: > On Sat, May 25, 2013 at 10:09 AM, Hans Petter Selasky > 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