Xen project Mailing List

Re: [Xen-devel] [PATCH v3 2/2] xen/vm-events: Move parts of monitor_domctl code to common-side.

To: Tamas K Lengyel <tamas@xxxxxxxxxxxxx>, Jan Beulich <JBeulich@xxxxxxxx>

From: Corneliu ZUZU <czuzu@xxxxxxxxxxxxxxx>

Date: Mon, 15 Feb 2016 18:59:04 +0200

Cc: Keir Fraser <keir@xxxxxxx>, Ian Campbell <ian.campbell@xxxxxxxxxx>, Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxx>, Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>

Comment: DomainKeys? See http://domainkeys.sourceforge.net/

Delivery-date: Mon, 15 Feb 2016 16:59:25 +0000

Domainkey-signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=bitdefender.com; b=YxBeaSDyUgjKgtYwbwDSn2CqG0pT2HBF5CQ0Hh50bDqDs0tuS0Oez2qP/3ObHgIo2usJOtIloOcu0hBR0BfFbFSmgSiGNLT8afxAhOmp2XkTmJozaRY1exn60xK9RIH+VJ9puguJi0Lt1SgYDhqzhGLeeXLWZInbeuG8CAOohwtTT50+yVcUqVTRIrqATUqjfJpn8aSO7lv/A4RBJmch55lDB2wzCeIiDNlDzRFd6VixNZM5+jWB/m6NE36X4tTQGljTIz5iySHMrcMBTFKSKpY3vwq3EnrFBIV2T++xbap8cgZOcGVJU3iAxYHL/YhtjOw8RyWAdfikQmAVR29NMg==; h=Received:Received:Received:Received:Received:Subject:To:References:Cc:From:Message-ID:Date:User-Agent:MIME-Version:In-Reply-To:Content-Type:X-BitDefender-Scanner:X-BitDefender-Spam:X-BitDefender-SpamStamp:X-BitDefender-CF-Stamp;

List-id: Xen developer discussion <xen-devel.lists.xen.org>

On 2/15/2016 6:51 PM, Tamas K Lengyel wrote:

On Mon, Feb 15, 2016 at 9:44 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:

>>> On 15.02.16 at 17:28, <czuzu@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxx http://lists.xen.org/xen-devel