On 2/15/2016 6:51 PM, Tamas K Lengyel wrote:
On Mon, Feb 15, 2016 at 9:44 AM, Jan Beulich <jbeul...@suse.com
<mailto:jbeul...@suse.com>> wrote:
>>> On 15.02.16 at 17:28, <cz...@bitdefender.com
<mailto:cz...@bitdefender.com>> wrote:
> On 2/15/2016 4:08 PM, Jan Beulich wrote:
>>
>>> After changing 1 to 1U though, I don't understand why we
should also
>>> range-check mop->event.
>>> I'm imagining when (mop->event > 31):
>>> * (1U << mop->event) = 0 or >= (0x1 + 0xFFFFFFFF) (?)
>> No, it's plain undefined.
>
> Weirdo C, didn't know that!
> I've just read
http://www.danielvik.com/2010/05/c-language-quirks.html .
> That's crazy, I can't believe such 'quirks' exist and that I
never knew
> of them.
> So then, would this do:
>
> /* sanity check - avoid '<<' operator undefined behavior */
> if ( unlikely(mop->event > 31) )
> return -EINVAL;
> if ( unlikely(!(arch_monitor_get_capabilities(d) & (1U <<
mop->event))) )
> return -EOPNOTSUPP;
I'd say -EOPNOTSUPP in both cases, but if the maintainers like
-EINVAL better I wouldn't insist on my preference.
The best approach of course would be if we had __MAX values defined
for such enums to check against but that doesn't seem to be part of
Xen's coding practice. So in this case I would say leave it as -EINVAL
as it's more descriptive of the problem and may even signal to the
caller some inherent bug on their side, not just that the requested
option is not supported.
Thanks,
Tamas
Good points, noted.
Corneliu.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
