Re: [PATCH] simplify pdpte check

Avi Kivity
2007-02-01T14:52:18Z

Muli Ben-Yehuda wrote:
> On Thu, Feb 01, 2007 at 04:07:01PM +0200, Avi Kivity wrote:
>
>> Muli Ben-Yehuda wrote:
>>
>>> Small cleanup: we check (pdpte & 1) && (pdpte & constant). Instead
>>> just check (pdpte & (constant | 1)).
>>>
>>> Signed-off-by: Muli Ben-Yehuda <muli-7z/5BgaJwgfQT0dZR+AlfA@public.gmane.org>
>>>
>>> Index: kernel/kvm_main.c
>>> ===================================================================
>>> --- kernel/kvm_main.c (revision 4379)
>>> +++ kernel/kvm_main.c (working copy)
>>> @@ -336,7 +336,7 @@
>>> ret = 1;
>>> for (i = 0; i < 4; ++i) {
>>> pdpte = pdpt[offset + i];
>>> - if ((pdpte & 1) && (pdpte & 0xfffffff0000001e6ull)) {
>>> + if (pdpte & 0xfffffff0000001e7ull) {
>>> ret = 0;
>>> goto out;
>>> }
>>>
>>>
>>>
>> The code is not equivalent. For example, pdpte == 2 passes the check
>> before the patch and fails it after.
>>
>
> err... 2 & 1 == 0, which does not pass the original check. Perhaps the
> real error is '&&' instead of '||' in the original conditional?
>
pdpte == 2 is a legal value. in the original code, the (pdpte & 1) == 0
and we don't 'goto out'. In the patched code, (pdpte & 0xblah) == 2 and
we 'goto out'.

if the code had '||' instead of '&&', your change would be correct, but
'&&' is really meant here.

--
error compiling committee.c: too many arguments to function