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