Re: [Xen-devel] [PATCH 2/3] x86/hvm: Rearange check_segment() to use a switch statement

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 03.07.17 at 15:15, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 03/07/17 13:34, Jan Beulich wrote:
>>>>> On 30.06.17 at 17:04, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> +    case x86_seg_ds:
>>> +    case x86_seg_es:
>>> +        if ( (reg->attr.fields.type & 0x8) && !(reg->attr.fields.type & 
>>> 0x2) )
>>> +        {
>>> +            gprintk(XENLOG_ERR, "Non-readable segment provided for DS or 
> ES\n");
>>> +            return -EINVAL;
>>> +        }
>>> +        break;
>>> +
>>> +    default: /* -Werror=switch */
>>> +        break;
>>>      }
>> Perhaps better to have
>>
>>     default:
>>         ASSERT_UNREACHABLE();
>>     case x86_seg_tr:
>>         break;
>>
>> to make more visible that it is not an oversight that especially FS
>> and GS aren't being handled here? Either way
>> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> The x86_seg_tr case exits check_segment() rather earlier.

I don't think it does - there are just two specific error paths there.

>  How about
> 
> default:
>     ASSERT_UNREACHABLE();
>     return -EINVAL;
> 
> ?

Indeed I would have suggested this if I had been able to convince
myself that x86_seg_tr can't come here.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel