From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Nrwqe-0006Jn-C3 for qemu-devel@nongnu.org; Wed, 17 Mar 2010 13:16:00 -0400 Received: from [199.232.76.173] (port=51171 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Nrwqd-0006JC-TD for qemu-devel@nongnu.org; Wed, 17 Mar 2010 13:15:59 -0400 Received: from Debian-exim by monty-python.gnu.org with spam-scanned (Exim 4.60) (envelope-from ) id 1Nrwqc-0005Ww-LV for qemu-devel@nongnu.org; Wed, 17 Mar 2010 13:15:59 -0400 Received: from mail-pv0-f173.google.com ([74.125.83.173]:44398) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1Nrwqc-0005Ws-9m for qemu-devel@nongnu.org; Wed, 17 Mar 2010 13:15:58 -0400 Received: by pvf33 with SMTP id 33so628224pvf.4 for ; Wed, 17 Mar 2010 10:15:56 -0700 (PDT) Message-ID: <4BA10E49.90703@codemonkey.ws> Date: Wed, 17 Mar 2010 12:15:53 -0500 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH] [TRIVIAL] usb-linux: remove unreachable default in switch statement References: <1268053115.2130.4.camel@localhost.localdomain> <1268842456.2231.6.camel@localhost.localdomain> <4BA105C7.6030806@codemonkey.ws> <201003171708.04633.paul@codesourcery.com> In-Reply-To: <201003171708.04633.paul@codesourcery.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paul Brook Cc: Paul Bolle , qemu-devel@nongnu.org On 03/17/2010 12:08 PM, Paul Brook wrote: >> On 03/17/2010 11:14 AM, Paul Bolle wrote: >> >>> On Wed, 2010-03-17 at 10:59 -0500, Anthony Liguori wrote: >>> >>>> On 03/08/2010 06:58 AM, Paul Bolle wrote: >>>> >>>>> Signed-off-by: Paul Bolle >>>>> >>>> Applied. Thanks. >>>> >>> Paul Brook was "tempted to replace it by an abort()" (about one and a >>> half week ago). Did you perhaps miss that message or weren't you tempted >>> to do this? >>> >> I missed it, but then again, I don't think the patch was wrong in the >> first place. >> >> I think we use too many aborts/exits in the device model that can >> potentially be triggered by guest code. >> > If something should never happen (as in this case) then an abort/assert is > completely appropriate. Once things get that screwed up there's no right > answer, and the best thing we can do is terminate immediately to try and avoid > further damage. > This case was: switch (foo & 0x03) { case 0: case 1: case 2: case 3: default: } The default is unreachable. Having it there just introduces more code that serves no purpose. Unless someone does something totally foolish and changes the mask in the switch statement, there's no way it will ever be reachable. > If an assert/abort can be triggered by a guest then you obviously have a bug. > Agreed. > Removing the assert is not the correct solution. You should either fix > whatever caused the invalid state to occur, or replace it with an appropriate > retry, fallback or guest visible failure. > Also agreed. Regards, Anthony Liguori > Paul >