From owner-svn-src-all@FreeBSD.ORG Sun May 26 02:45:49 2013 Return-Path: Delivered-To: svn-src-all@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 69C921C7; Sun, 26 May 2013 02:45:49 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from fallbackmx08.syd.optusnet.com.au (fallbackmx08.syd.optusnet.com.au [211.29.132.10]) by mx1.freebsd.org (Postfix) with ESMTP id 02BF3399; Sun, 26 May 2013 02:45:48 +0000 (UTC) Received: from mail35.syd.optusnet.com.au (mail35.syd.optusnet.com.au [211.29.133.51]) by fallbackmx08.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id r4Q2jXLx027840; Sun, 26 May 2013 12:45:33 +1000 Received: from c122-106-156-23.carlnfd1.nsw.optusnet.com.au (c122-106-156-23.carlnfd1.nsw.optusnet.com.au [122.106.156.23]) by mail35.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id r4Q2jIai023179 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sun, 26 May 2013 12:45:21 +1000 Date: Sun, 26 May 2013 12:45:18 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: mdf@FreeBSD.org Subject: Re: svn commit: r250986 - head/sys/dev/usb In-Reply-To: Message-ID: <20130526122502.P847@besplex.bde.org> References: <201305251709.r4PH9xfv015285@svn.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.0 cv=e/de0tV/ c=1 sm=1 a=sQw_kobpeVwA:10 a=kj9zAlcOel0A:10 a=PO7r1zJSAAAA:8 a=JzwRw_2MAAAA:8 a=FG3D6h2hVzcA:10 a=6I5d2MoRAAAA:8 a=6Rb0blR-ypmRijCZwjYA:9 a=CjuIK1q_8ugA:10 a=SV7veod9ZcQA:10 a=ebeQFi2P/qHVC0Yw9JDJ4g==:117 Cc: "svn-src-head@freebsd.org" , "svn-src-all@freebsd.org" , "src-committers@freebsd.org" , Hans Petter Selasky 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: Sun, 26 May 2013 02:45:49 -0000 On Sat, 25 May 2013 mdf@FreeBSD.org wrote: > On Sat, May 25, 2013 at 10:09 AM, Hans Petter Selasky > wrote: >> ... >> Log: >> Fix some statical clang analyzer warnings. >> ... >> ============================================================================== >> --- 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). No, initializing a variable early to a sane value obfuscates the code and makes it impossible for the compiler to detect that the variable is not properly intialized. Initializing early to an insane value that will be overwritten in all cases is not so bad. This makes it clear to human readers that the initial value should not be used, and gives runtime errors if it is used (best an immediate trap), but still prevents the compiler detecting that the variable is not properly initialized. Hmm, it would be useful to have a compiler flag for initializing all local variables to trap representations on entry to functions. This gives the runtime check in addition to the compiler check, without writing huge code to initialize all the variables. Then early initialization would break to sane values would break this feature. Of course, in practical code, you often reuse a variable without uninitializing it (by setting it to an insane value) after each of its uses becomes dead. Then you lose the compiler check. If the uses are unrelated, then it is a style bug (optimization for 30 year old compilers with no lifetime analysis) to use the same variable for them all. Otherwise, it is too painful to uninitialize variables or use block scope for them to make their lifetimes more obvious to all readers. One exception is when pointer variables are set to NULL after they are freed even when the pointers are not expected to be reused. This is done mainly for non-local variables. Bruce