Subject: Re: This looks wrong...
To: None <port-i386@NetBSD.ORG>
From: Christos Zoulas <christos@zoulas.com>
List: port-i386
Date: 11/11/1997 06:42:01
In article <27910.878927747@dnaunix.pa.dec.com> cgd@pa.dec.com (Chris G. Demetriou) writes:
>> >This is wrong, isn't it?
>>
>> >This is in i386/pmap.c, pmap_changebit():
>> >
>> > /*
>> > * XXX don't write protect pager mappings
>> > */
>> > if ((PG_RO && setbits == PG_RO) ||
>> > (PG_RW && maskbits == ~PG_RW)) {
>> > extern vm_offset_t pager_sva, pager_eva;
>> >
>> > if (va >= pager_sva && va < pager_eva)
>> > continue;
>> > }
>>
>> >Shouldn't the &&s in the first if be &?
>>
>> it is ok. note from pte.h:
>> #define PG_RO 0x00000000 /* read-only by user (and kernel if 486) */
>> #define PG_RW 0x00000002 /* read-write by user */
>>
>> you have to be careful with PG_RO because it is zero (testing for
>> "setbits == 0" doesn't mean you are write protecting).
>>
>> the compiler should reduce the if to:
>>
>> if (maskbits == ~PG_RW) { /* do pager VA hack test... */ }
>
>Some would say that, at the very least, the bug here is that the
>comment doesn't describe the trickery being used (if that trickerly is
>indeed the intent)...
we should definitely fix the code so that it is readable...
christos