Subject: Re: This looks wrong...
To: Chuck Cranor <chuck@dworkin.wustl.edu>
From: Chris G. Demetriou <cgd@pa.dec.com>
List: port-i386
Date: 11/07/1997 10:35:47
> >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)...


cgd