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