[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 2/3] x86/hvm: Rearange check_segment() to use a switch statement
>>> On 03.07.17 at 15:58, <andrew.cooper3@xxxxxxxxxx> wrote: > On 03/07/17 14:33, Jan Beulich wrote: >>>>> 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. > > You are quite right. I was mistaken. I will go with this: > > case x86_seg_tr: > break; > > default: > ASSERT_UNREACHABLE(); > return -EINVAL; > } > > which fails slightly safer than your suggestion in release builds. Ah, yes, agreed. Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |