qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Anthony Liguori <anthony@codemonkey.ws>
To: Paul Brook <paul@codesourcery.com>
Cc: Paul Bolle <pebolle@tiscali.nl>, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH] [TRIVIAL] usb-linux: remove unreachable default in switch statement
Date: Wed, 17 Mar 2010 12:15:53 -0500	[thread overview]
Message-ID: <4BA10E49.90703@codemonkey.ws> (raw)
In-Reply-To: <201003171708.04633.paul@codesourcery.com>

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<pebolle@tiscali.nl>
>>>>>            
>>>> 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
>    

  reply	other threads:[~2010-03-17 17:16 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-03-08 12:58 [Qemu-devel] [PATCH] [TRIVIAL] usb-linux: remove unreachable default in switch statement Paul Bolle
2010-03-08 13:31 ` Paul Brook
2010-03-17 15:59 ` Anthony Liguori
2010-03-17 16:14   ` Paul Bolle
2010-03-17 16:39     ` Anthony Liguori
2010-03-17 17:08       ` Paul Brook
2010-03-17 17:15         ` Anthony Liguori [this message]
2010-03-17 17:43           ` Paul Brook
2010-03-17 20:15             ` Blue Swirl
2010-03-17 20:41               ` Anthony Liguori
2010-03-17 20:56                 ` Paul Bolle
2010-03-17 20:59                   ` Anthony Liguori
2010-03-17 21:05                     ` Blue Swirl

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4BA10E49.90703@codemonkey.ws \
    --to=anthony@codemonkey.ws \
    --cc=paul@codesourcery.com \
    --cc=pebolle@tiscali.nl \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).