From 1a81e38b17144624415d252a521fd5a06079d681 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Tue, 11 Jan 2011 13:01:13 -0500 Subject: [PATCH] make new code like old code Variable declarations at top of function, separate from initialization. Use == 0 instead of ! for checking pointers. Consistent spacing around {, *, casts. Declare 0-parameter functions as (void) not (). Integer valued functions return -1 on failure, 0 on success. --- bootmain.c | 2 +- console.c | 5 +- defs.h | 8 +-- exec.c | 24 ++++----- ide.c | 4 +- kalloc.c | 14 ++--- main.c | 30 +++++------ mkfs.c | 24 ++++----- mmu.h | 14 ++--- mp.c | 4 +- pipe.c | 4 +- proc.c | 12 +++-- spinlock.c | 2 +- stressfs.c | 13 +++-- syscall.c | 9 ++-- sysfile.c | 2 +- ulib.c | 2 +- umalloc.c | 6 +-- user.h | 4 +- usertests.c | 100 ++++++++++++++++++------------------ vm.c | 143 ++++++++++++++++++++++++++++++---------------------- 21 files changed, 227 insertions(+), 199 deletions(-) diff --git a/bootmain.c b/bootmain.c index f67f8fa..7cd469f 100644 --- a/bootmain.c +++ b/bootmain.c @@ -33,7 +33,7 @@ bootmain(void) // Load each program segment (ignores ph flags). ph = (struct proghdr*)((uchar*)elf + elf->phoff); eph = ph + elf->phnum; - for(; ph < eph; ph++) { + for(; ph < eph; ph++){ va = (uchar*)ph->va; readseg(va, ph->filesz, ph->offset); if(ph->memsz > ph->filesz) diff --git a/console.c b/console.c index 19b3296..634e9e8 100644 --- a/console.c +++ b/console.c @@ -27,15 +27,16 @@ printint(int xx, int base, int sgn) { static char digits[] = "0123456789abcdef"; char buf[16]; - int i = 0, neg = 0; + int i, neg; uint x; - if(sgn && xx < 0){ + if(sgn && (neg = xx < 0)){ neg = 1; x = -xx; } else x = xx; + i = 0; do{ buf[i++] = digits[x % base]; }while((x /= base) != 0); diff --git a/defs.h b/defs.h index d08609d..43f35d2 100644 --- a/defs.h +++ b/defs.h @@ -62,7 +62,7 @@ void ioapicinit(void); // kalloc.c char* kalloc(void); void kfree(char*); -void kinit(); +void kinit(void); // kbd.c void kbdintr(void); @@ -117,8 +117,8 @@ void getcallerpcs(void*, uint*); int holding(struct spinlock*); void initlock(struct spinlock*, char*); void release(struct spinlock*); -void pushcli(); -void popcli(); +void pushcli(void); +void popcli(void); // string.c int memcmp(const void*, const void*, uint); @@ -164,7 +164,7 @@ void inituvm(pde_t*, char*, uint); int loaduvm(pde_t*, char*, struct inode *, uint, uint); pde_t* copyuvm(pde_t*,uint); void switchuvm(struct proc*); -void switchkvm(); +void switchkvm(void); int copyout(pde_t *pgdir, uint va, void *buf, uint len); // number of elements in fixed-size array diff --git a/exec.c b/exec.c index 1673a38..209bc79 100644 --- a/exec.c +++ b/exec.c @@ -10,16 +10,17 @@ int exec(char *path, char **argv) { char *s, *last; - int i, off; - uint sz = 0; + int i, off, argc; + uint sz, sp, strings[MAXARG]; struct elfhdr elf; - struct inode *ip = 0; + struct inode *ip; struct proghdr ph; - pde_t *pgdir = 0, *oldpgdir; + pde_t *pgdir, *oldpgdir; if((ip = namei(path)) == 0) return -1; ilock(ip); + pgdir = 0; // Check ELF header if(readi(ip, (char*)&elf, 0, sizeof(elf)) < sizeof(elf)) @@ -27,10 +28,11 @@ exec(char *path, char **argv) if(elf.magic != ELF_MAGIC) goto bad; - if(!(pgdir = setupkvm())) + if((pgdir = setupkvm()) == 0) goto bad; // Load program into memory. + sz = 0; for(i=0, off=elf.phoff; i= MAXARG) goto bad; // push strings and remember where they are - uint strings[MAXARG]; for(i = argc - 1; i >= 0; --i){ sp -= strlen(argv[i]) + 1; strings[i] = sp; copyout(pgdir, sp, argv[i], strlen(argv[i]) + 1); } -#define PUSH(x) { int xx = (int)(x); sp -= 4; copyout(pgdir, sp, &xx, 4); } +#define PUSH(x){ int xx = (int)(x); sp -= 4; copyout(pgdir, sp, &xx, 4); } PUSH(0); // argv[argc] is zero diff --git a/ide.c b/ide.c index 7b12aa0..f4bd210 100644 --- a/ide.c +++ b/ide.c @@ -131,7 +131,7 @@ iderw(struct buf *b) if((b->flags & (B_VALID|B_DIRTY)) == B_VALID) panic("iderw: nothing to do"); if(b->dev != 0 && !havedisk1) - panic("idrw: ide disk 1 not present"); + panic("iderw: ide disk 1 not present"); acquire(&idelock); @@ -147,7 +147,7 @@ iderw(struct buf *b) // Wait for request to finish. // Assuming will not sleep too long: ignore proc->killed. - while((b->flags & (B_VALID|B_DIRTY)) != B_VALID) { + while((b->flags & (B_VALID|B_DIRTY)) != B_VALID){ sleep(b, &idelock); } diff --git a/kalloc.c b/kalloc.c index 72ce58a..e31d71d 100644 --- a/kalloc.c +++ b/kalloc.c @@ -23,9 +23,11 @@ extern char end[]; // first address after kernel loaded from ELF file void kinit(void) { + char *p; + initlock(&kmem.lock, "kmem"); - char *p = (char*)PGROUNDUP((uint)end); - for( ; p + PGSIZE - 1 < (char*) PHYSTOP; p += PGSIZE) + p = (char*)PGROUNDUP((uint)end); + for(; p + PGSIZE - 1 < (char*)PHYSTOP; p += PGSIZE) kfree(p); } @@ -39,14 +41,14 @@ kfree(char *v) { struct run *r; - if(((uint) v) % PGSIZE || v < end || (uint)v >= PHYSTOP) + if((uint)v % PGSIZE || v < end || (uint)v >= PHYSTOP) panic("kfree"); // Fill with junk to catch dangling refs. memset(v, 1, PGSIZE); acquire(&kmem.lock); - r = (struct run *) v; + r = (struct run*)v; r->next = kmem.freelist; kmem.freelist = r; release(&kmem.lock); @@ -56,7 +58,7 @@ kfree(char *v) // Returns a pointer that the kernel can use. // Returns 0 if the memory cannot be allocated. char* -kalloc() +kalloc(void) { struct run *r; @@ -65,6 +67,6 @@ kalloc() if(r) kmem.freelist = r->next; release(&kmem.lock); - return (char*) r; + return (char*)r; } diff --git a/main.c b/main.c index beac4da..8a5f7f1 100644 --- a/main.c +++ b/main.c @@ -7,7 +7,7 @@ static void bootothers(void); static void mpmain(void); -void jkstack(void) __attribute__((noreturn)); +void jmpkstack(void) __attribute__((noreturn)); void mainc(void); // Bootstrap processor starts running C code here. @@ -20,19 +20,20 @@ main(void) lapicinit(mpbcpu()); seginit(); // set up segments kinit(); // initialize memory allocator - jkstack(); // call mainc() on a properly-allocated stack + jmpkstack(); // call mainc() on a properly-allocated stack } void -jkstack(void) +jmpkstack(void) { - char *kstack = kalloc(); - if(!kstack) - panic("jkstack\n"); - char *top = kstack + PGSIZE; - asm volatile("movl %0,%%esp" : : "r" (top)); - asm volatile("call mainc"); - panic("jkstack"); + char *kstack, *top; + + kstack = kalloc(); + if(kstack == 0) + panic("jmpkstack kalloc"); + top = kstack + PGSIZE; + asm volatile("movl %0,%%esp; call mainc" : : "r" (top)); + panic("jmpkstack"); } // Set up hardware and software. @@ -67,7 +68,7 @@ mainc(void) static void mpmain(void) { - if(cpunum() != mpbcpu()) { + if(cpunum() != mpbcpu()){ seginit(); lapicinit(cpunum()); } @@ -87,9 +88,9 @@ bootothers(void) struct cpu *c; char *stack; - // Write bootstrap code to unused memory at 0x7000. The linker has - // placed the start of bootother.S there. - code = (uchar *) 0x7000; + // Write bootstrap code to unused memory at 0x7000. + // The linker has placed the image of bootother.S in _binary_bootother_start. + code = (uchar*)0x7000; memmove(code, _binary_bootother_start, (uint)_binary_bootother_size); for(c = cpus; c < cpus+ncpu; c++){ @@ -110,4 +111,3 @@ bootothers(void) ; } } - diff --git a/mkfs.c b/mkfs.c index ffd6745..77e6791 100644 --- a/mkfs.c +++ b/mkfs.c @@ -35,7 +35,7 @@ ushort xshort(ushort x) { ushort y; - uchar *a = (uchar*) &y; + uchar *a = (uchar*)&y; a[0] = x; a[1] = x >> 8; return y; @@ -45,7 +45,7 @@ uint xint(uint x) { uint y; - uchar *a = (uchar*) &y; + uchar *a = (uchar*)&y; a[0] = x; a[1] = x >> 8; a[2] = x >> 16; @@ -177,7 +177,7 @@ winode(uint inum, struct dinode *ip) bn = i2b(inum); rsect(bn, buf); - dip = ((struct dinode*) buf) + (inum % IPB); + dip = ((struct dinode*)buf) + (inum % IPB); *dip = *ip; wsect(bn, buf); } @@ -191,7 +191,7 @@ rinode(uint inum, struct dinode *ip) bn = i2b(inum); rsect(bn, buf); - dip = ((struct dinode*) buf) + (inum % IPB); + dip = ((struct dinode*)buf) + (inum % IPB); *ip = *dip; } @@ -231,7 +231,7 @@ balloc(int used) printf("balloc: first %d blocks have been allocated\n", used); assert(used < 512); bzero(buf, 512); - for(i = 0; i < used; i++) { + for(i = 0; i < used; i++){ buf[i/8] = buf[i/8] | (0x1 << (i%8)); } printf("balloc: write bitmap block at sector %zu\n", ninodes/IPB + 3); @@ -243,7 +243,7 @@ balloc(int used) void iappend(uint inum, void *xp, int n) { - char *p = (char*) xp; + char *p = (char*)xp; uint fbn, off, n1; struct dinode din; char buf[512]; @@ -256,24 +256,24 @@ iappend(uint inum, void *xp, int n) while(n > 0){ fbn = off / 512; assert(fbn < MAXFILE); - if(fbn < NDIRECT) { - if(xint(din.addrs[fbn]) == 0) { + if(fbn < NDIRECT){ + if(xint(din.addrs[fbn]) == 0){ din.addrs[fbn] = xint(freeblock++); usedblocks++; } x = xint(din.addrs[fbn]); } else { - if(xint(din.addrs[NDIRECT]) == 0) { + if(xint(din.addrs[NDIRECT]) == 0){ // printf("allocate indirect block\n"); din.addrs[NDIRECT] = xint(freeblock++); usedblocks++; } // printf("read indirect block\n"); - rsect(xint(din.addrs[NDIRECT]), (char*) indirect); - if(indirect[fbn - NDIRECT] == 0) { + rsect(xint(din.addrs[NDIRECT]), (char*)indirect); + if(indirect[fbn - NDIRECT] == 0){ indirect[fbn - NDIRECT] = xint(freeblock++); usedblocks++; - wsect(xint(din.addrs[NDIRECT]), (char*) indirect); + wsect(xint(din.addrs[NDIRECT]), (char*)indirect); } x = xint(indirect[fbn-NDIRECT]); } diff --git a/mmu.h b/mmu.h index 475eae8..2d88a52 100644 --- a/mmu.h +++ b/mmu.h @@ -98,18 +98,18 @@ struct segdesc { // \--- PDX(la) --/ \--- PTX(la) --/ // page directory index -#define PDX(la) ((((uint) (la)) >> PDXSHIFT) & 0x3FF) +#define PDX(la) (((uint)(la) >> PDXSHIFT) & 0x3FF) // page table index -#define PTX(la) ((((uint) (la)) >> PTXSHIFT) & 0x3FF) +#define PTX(la) (((uint)(la) >> PTXSHIFT) & 0x3FF) // construct linear address from indexes and offset -#define PGADDR(d, t, o) ((uint) ((d) << PDXSHIFT | (t) << PTXSHIFT | (o))) +#define PGADDR(d, t, o) ((uint)((d) << PDXSHIFT | (t) << PTXSHIFT | (o))) // turn a kernel linear address into a physical address. // all of the kernel data structures have linear and // physical addresses that are equal. -#define PADDR(a) ((uint) a) +#define PADDR(a) ((uint)(a)) // Page directory and page table constants. #define NPDENTRIES 1024 // page directory entries per page directory @@ -136,7 +136,7 @@ struct segdesc { #define PTE_MBZ 0x180 // Bits must be zero // Address in page table or page directory entry -#define PTE_ADDR(pte) ((uint) (pte) & ~0xFFF) +#define PTE_ADDR(pte) ((uint)(pte) & ~0xFFF) typedef uint pte_t; @@ -205,7 +205,7 @@ struct gatedesc { // this interrupt/trap gate explicitly using an int instruction. #define SETGATE(gate, istrap, sel, off, d) \ { \ - (gate).off_15_0 = (uint) (off) & 0xffff; \ + (gate).off_15_0 = (uint)(off) & 0xffff; \ (gate).cs = (sel); \ (gate).args = 0; \ (gate).rsv1 = 0; \ @@ -213,6 +213,6 @@ struct gatedesc { (gate).s = 0; \ (gate).dpl = (d); \ (gate).p = 1; \ - (gate).off_31_16 = (uint) (off) >> 16; \ + (gate).off_31_16 = (uint)(off) >> 16; \ } diff --git a/mp.c b/mp.c index d2f828a..edabdd6 100644 --- a/mp.c +++ b/mp.c @@ -113,8 +113,8 @@ mpinit(void) switch(*p){ case MPPROC: proc = (struct mpproc*)p; - if(ncpu != proc->apicid) { - cprintf("mpinit: ncpu=%d apicpid=%d", ncpu, proc->apicid); + if(ncpu != proc->apicid){ + cprintf("mpinit: ncpu=%d apicpid=%d\n", ncpu, proc->apicid); panic("mpinit"); } if(proc->flags & MPBOOT) diff --git a/pipe.c b/pipe.c index bc847b9..f76ed5c 100644 --- a/pipe.c +++ b/pipe.c @@ -66,7 +66,7 @@ pipeclose(struct pipe *p, int writable) p->readopen = 0; wakeup(&p->nwrite); } - if(p->readopen == 0 && p->writeopen == 0) { + if(p->readopen == 0 && p->writeopen == 0){ release(&p->lock); kfree((char*)p); } else @@ -81,7 +81,7 @@ pipewrite(struct pipe *p, char *addr, int n) acquire(&p->lock); for(i = 0; i < n; i++){ - while(p->nwrite == p->nread + PIPESIZE) { //DOC: pipewrite-full + while(p->nwrite == p->nread + PIPESIZE){ //DOC: pipewrite-full if(p->readopen == 0 || proc->killed){ release(&p->lock); return -1; diff --git a/proc.c b/proc.c index 2e8a0a4..e6ccd9d 100644 --- a/proc.c +++ b/proc.c @@ -120,7 +120,7 @@ userinit(void) p = allocproc(); initproc = p; - if(!(p->pgdir = setupkvm())) + if((p->pgdir = setupkvm()) == 0) panic("userinit: out of memory?"); inituvm(p->pgdir, _binary_initcode_start, (int)_binary_initcode_size); p->sz = PGSIZE; @@ -144,12 +144,14 @@ userinit(void) int growproc(int n) { - uint sz = proc->sz; + uint sz; + + sz = proc->sz; if(n > 0){ - if(!(sz = allocuvm(proc->pgdir, sz, sz + n))) + if((sz = allocuvm(proc->pgdir, sz, sz + n)) == 0) return -1; } else if(n < 0){ - if(!(sz = deallocuvm(proc->pgdir, sz, sz + n))) + if((sz = deallocuvm(proc->pgdir, sz, sz + n)) == 0) return -1; } proc->sz = sz; @@ -171,7 +173,7 @@ fork(void) return -1; // Copy process state from p. - if(!(np->pgdir = copyuvm(proc->pgdir, proc->sz))){ + if((np->pgdir = copyuvm(proc->pgdir, proc->sz)) == 0){ kfree(np->kstack); np->kstack = 0; np->state = UNUSED; diff --git a/spinlock.c b/spinlock.c index 281748a..e668598 100644 --- a/spinlock.c +++ b/spinlock.c @@ -71,7 +71,7 @@ getcallerpcs(void *v, uint pcs[]) ebp = (uint*)v - 2; for(i = 0; i < 10; i++){ - if(ebp == 0 || ebp < (uint *) 0x100000 || ebp == (uint*)0xffffffff) + if(ebp == 0 || ebp < (uint*)0x100000 || ebp == (uint*)0xffffffff) break; pcs[i] = ebp[1]; // saved %eip ebp = (uint*)ebp[0]; // saved %ebp diff --git a/stressfs.c b/stressfs.c index 43f312b..5d4fee2 100644 --- a/stressfs.c +++ b/stressfs.c @@ -14,20 +14,19 @@ int main(int argc, char *argv[]) { - int i; + int fd, i; + char path[] = "stressfs0"; + printf(1, "stressfs starting\n"); - for(i = 0; i < 4; i++){ - if(fork() > 0){ + for(i = 0; i < 4; i++) + if(fork() > 0) break; - } - } printf(1, "%d\n", i); - char path[] = "stressfs0"; path[8] += i; - int fd = open(path, O_CREATE | O_RDWR); + fd = open(path, O_CREATE | O_RDWR); for(i = 0; i < 100; i++) printf(fd, "%d\n", i); close(fd); diff --git a/syscall.c b/syscall.c index f8aeae7..16c5b47 100644 --- a/syscall.c +++ b/syscall.c @@ -32,8 +32,8 @@ fetchstr(struct proc *p, uint addr, char **pp) if(addr >= p->sz) return -1; - *pp = (char *) addr; - ep = (char *) p->sz; + *pp = (char*)addr; + ep = (char*)p->sz; for(s = *pp; s < ep; s++) if(*s == 0) return s - *pp; @@ -44,8 +44,7 @@ fetchstr(struct proc *p, uint addr, char **pp) int argint(int n, int *ip) { - int x = fetchint(proc, proc->tf->esp + 4 + 4*n, ip); - return x; + return fetchint(proc, proc->tf->esp + 4 + 4*n, ip); } // Fetch the nth word-sized system call argument as a pointer @@ -60,7 +59,7 @@ argptr(int n, char **pp, int size) return -1; if((uint)i >= proc->sz || (uint)i+size >= proc->sz) return -1; - *pp = (char *) i; + *pp = (char*)i; return 0; } diff --git a/sysfile.c b/sysfile.c index 0b42920..4235660 100644 --- a/sysfile.c +++ b/sysfile.c @@ -348,7 +348,7 @@ sys_exec(void) int i; uint uargv, uarg; - if(argstr(0, &path) < 0 || argint(1, (int*)&uargv) < 0) { + if(argstr(0, &path) < 0 || argint(1, (int*)&uargv) < 0){ return -1; } memset(argv, 0, sizeof(argv)); diff --git a/ulib.c b/ulib.c index 0268c26..dbbcfcf 100644 --- a/ulib.c +++ b/ulib.c @@ -45,7 +45,7 @@ strchr(const char *s, char c) { for(; *s; s++) if(*s == c) - return (char*) s; + return (char*)s; return 0; } diff --git a/umalloc.c b/umalloc.c index 4984591..a7e7d2c 100644 --- a/umalloc.c +++ b/umalloc.c @@ -26,7 +26,7 @@ free(void *ap) { Header *bp, *p; - bp = (Header*) ap - 1; + bp = (Header*)ap - 1; for(p = freep; !(bp > p && bp < p->s.ptr); p = p->s.ptr) if(p >= p->s.ptr && (bp > p || bp < p->s.ptr)) break; @@ -52,7 +52,7 @@ morecore(uint nu) if(nu < 4096) nu = 4096; p = sbrk(nu * sizeof(Header)); - if(p == (char*) -1) + if(p == (char*)-1) return 0; hp = (Header*)p; hp->s.size = nu; @@ -81,7 +81,7 @@ malloc(uint nbytes) p->s.size = nunits; } freep = prevp; - return (void*) (p + 1); + return (void*)(p + 1); } if(p == freep) if((p = morecore(nunits)) == 0) diff --git a/user.h b/user.h index 431428c..9e26cf1 100644 --- a/user.h +++ b/user.h @@ -18,10 +18,10 @@ int link(char*, char*); int mkdir(char*); int chdir(char*); int dup(int); -int getpid(); +int getpid(void); char* sbrk(int); int sleep(int); -int uptime(); +int uptime(void); // ulib.c int stat(char*, struct stat*); diff --git a/usertests.c b/usertests.c index 5d1d8ea..9a83591 100644 --- a/usertests.c +++ b/usertests.c @@ -47,12 +47,12 @@ writetest(void) printf(stdout, "error: creat small failed!\n"); exit(); } - for(i = 0; i < 100; i++) { - if(write(fd, "aaaaaaaaaa", 10) != 10) { + for(i = 0; i < 100; i++){ + if(write(fd, "aaaaaaaaaa", 10) != 10){ printf(stdout, "error: write aa %d new file failed\n", i); exit(); } - if(write(fd, "bbbbbbbbbb", 10) != 10) { + if(write(fd, "bbbbbbbbbb", 10) != 10){ printf(stdout, "error: write bb %d new file failed\n", i); exit(); } @@ -67,7 +67,7 @@ writetest(void) exit(); } i = read(fd, buf, 2000); - if(i == 2000) { + if(i == 2000){ printf(stdout, "read succeeded ok\n"); } else { printf(stdout, "read failed\n"); @@ -75,7 +75,7 @@ writetest(void) } close(fd); - if(unlink("small") < 0) { + if(unlink("small") < 0){ printf(stdout, "unlink small failed\n"); exit(); } @@ -95,9 +95,9 @@ writetest1(void) exit(); } - for(i = 0; i < MAXFILE; i++) { - ((int*) buf)[0] = i; - if(write(fd, buf, 512) != 512) { + for(i = 0; i < MAXFILE; i++){ + ((int*)buf)[0] = i; + if(write(fd, buf, 512) != 512){ printf(stdout, "error: write big file failed\n", i); exit(); } @@ -112,19 +112,19 @@ writetest1(void) } n = 0; - for(;;) { + for(;;){ i = read(fd, buf, 512); - if(i == 0) { - if(n == MAXFILE - 1) { + if(i == 0){ + if(n == MAXFILE - 1){ printf(stdout, "read only %d blocks from big", n); exit(); } break; - } else if(i != 512) { + } else if(i != 512){ printf(stdout, "read failed %d\n", i); exit(); } - if(((int*)buf)[0] != n) { + if(((int*)buf)[0] != n){ printf(stdout, "read content of block %d is %d\n", n, ((int*)buf)[0]); exit(); @@ -132,7 +132,7 @@ writetest1(void) n++; } close(fd); - if(unlink("big") < 0) { + if(unlink("big") < 0){ printf(stdout, "unlink big failed\n"); exit(); } @@ -148,14 +148,14 @@ createtest(void) name[0] = 'a'; name[2] = '\0'; - for(i = 0; i < 52; i++) { + for(i = 0; i < 52; i++){ name[1] = '0' + i; fd = open(name, O_CREATE|O_RDWR); close(fd); } name[0] = 'a'; name[2] = '\0'; - for(i = 0; i < 52; i++) { + for(i = 0; i < 52; i++){ name[1] = '0' + i; unlink(name); } @@ -166,22 +166,22 @@ void dirtest(void) { printf(stdout, "mkdir test\n"); - if(mkdir("dir0") < 0) { + if(mkdir("dir0") < 0){ printf(stdout, "mkdir failed\n"); exit(); } - if(chdir("dir0") < 0) { + if(chdir("dir0") < 0){ printf(stdout, "chdir dir0 failed\n"); exit(); } - if(chdir("..") < 0) { + if(chdir("..") < 0){ printf(stdout, "chdir .. failed\n"); exit(); } - if(unlink("dir0") < 0) { + if(unlink("dir0") < 0){ printf(stdout, "unlink dir0 failed\n"); exit(); } @@ -192,7 +192,7 @@ void exectest(void) { printf(stdout, "exec test\n"); - if(exec("echo", echoargv) < 0) { + if(exec("echo", echoargv) < 0){ printf(stdout, "exec echo failed\n"); exit(); } @@ -330,17 +330,17 @@ mem(void) ppid = getpid(); if((pid = fork()) == 0){ m1 = 0; - while((m2 = malloc(10001)) != 0) { - *(char**) m2 = m1; + while((m2 = malloc(10001)) != 0){ + *(char**)m2 = m1; m1 = m2; } - while(m1) { + while(m1){ m2 = *(char**)m1; free(m1); m1 = m2; } m1 = malloc(1024*20); - if(m1 == 0) { + if(m1 == 0){ printf(1, "couldn't allocate mem?!!\n"); kill(ppid); exit(); @@ -1237,16 +1237,18 @@ forktest(void) void sbrktest(void) { - int pid; - char *oldbrk = sbrk(0); + int fds[2], pid, pids[32], ppid; + char *a, *b, *c, *lastaddr, *oldbrk, *p, scratch; + uint amt; printf(stdout, "sbrk test\n"); + oldbrk = sbrk(0); // can one sbrk() less than a page? - char *a = sbrk(0); + a = sbrk(0); int i; for(i = 0; i < 5000; i++){ - char *b = sbrk(1); + b = sbrk(1); if(b != a){ printf(stdout, "sbrk test failed %d %x %x\n", i, a, b); exit(); @@ -1259,7 +1261,7 @@ sbrktest(void) printf(stdout, "sbrk test fork failed\n"); exit(); } - char *c = sbrk(1); + c = sbrk(1); c = sbrk(1); if(c != a + 1){ printf(stdout, "sbrk test failed post-fork\n"); @@ -1271,18 +1273,18 @@ sbrktest(void) // can one allocate the full 640K? a = sbrk(0); - uint amt = (640 * 1024) - (uint) a; - char *p = sbrk(amt); + amt = (640 * 1024) - (uint)a; + p = sbrk(amt); if(p != a){ printf(stdout, "sbrk test failed 640K test, p %x a %x\n", p, a); exit(); } - char *lastaddr = (char *)(640 * 1024 - 1); + lastaddr = (char*)(640 * 1024 - 1); *lastaddr = 99; // is one forbidden from allocating more than 640K? c = sbrk(4096); - if(c != (char *) 0xffffffff){ + if(c != (char*)0xffffffff){ printf(stdout, "sbrk allocated more than 640K, c %x\n", c); exit(); } @@ -1290,7 +1292,7 @@ sbrktest(void) // can one de-allocate? a = sbrk(0); c = sbrk(-4096); - if(c == (char *) 0xffffffff){ + if(c == (char*)0xffffffff){ printf(stdout, "sbrk could not deallocate\n"); exit(); } @@ -1314,15 +1316,15 @@ sbrktest(void) } c = sbrk(4096); - if(c != (char *) 0xffffffff){ + if(c != (char*)0xffffffff){ printf(stdout, "sbrk was able to re-allocate beyond 640K, c %x\n", c); exit(); } // can we read the kernel's memory? - for(a = (char*)(640*1024); a < (char *)2000000; a += 50000){ - int ppid = getpid(); - int pid = fork(); + for(a = (char*)(640*1024); a < (char*)2000000; a += 50000){ + ppid = getpid(); + pid = fork(); if(pid < 0){ printf(stdout, "fork failed\n"); exit(); @@ -1338,21 +1340,18 @@ sbrktest(void) // if we run the system out of memory, does it clean up the last // failed allocation? sbrk(-(sbrk(0) - oldbrk)); - int pids[32]; - int fds[2]; if(pipe(fds) != 0){ printf(1, "pipe() failed\n"); exit(); } for(i = 0; i < sizeof(pids)/sizeof(pids[0]); i++){ - if((pids[i] = fork()) == 0) { + if((pids[i] = fork()) == 0){ // allocate the full 640K sbrk((640 * 1024) - (uint)sbrk(0)); write(fds[1], "x", 1); // sit around until killed for(;;) sleep(1000); } - char scratch; if(pids[i] != -1) read(fds[0], &scratch, 1); } @@ -1365,7 +1364,7 @@ sbrktest(void) kill(pids[i]); wait(); } - if(c == (char*)0xffffffff) { + if(c == (char*)0xffffffff){ printf(stdout, "failed sbrk leaked memory\n"); exit(); } @@ -1392,14 +1391,14 @@ validateint(int *p) void validatetest(void) { - int hi = 1100*1024; + int hi, pid; + uint p; printf(stdout, "validate test\n"); + hi = 1100*1024; - uint p; - for (p = 0; p <= (uint)hi; p += 4096) { - int pid; - if ((pid = fork()) == 0) { + for(p = 0; p <= (uint)hi; p += 4096){ + if((pid = fork()) == 0){ // try to crash the kernel by passing in a badly placed integer validateint((int*)p); exit(); @@ -1410,7 +1409,7 @@ validatetest(void) wait(); // try to crash the kernel by passing in a bad string pointer - if (link("nosuchfile", (char*)p) != -1) { + if(link("nosuchfile", (char*)p) != -1){ printf(stdout, "link should not succeed\n"); exit(); } @@ -1425,6 +1424,7 @@ void bsstest(void) { int i; + printf(stdout, "bss test\n"); for(i = 0; i < sizeof(uninit); i++){ if(uninit[i] != '\0'){ diff --git a/vm.c b/vm.c index 551efbf..940cc2f 100644 --- a/vm.c +++ b/vm.c @@ -42,23 +42,21 @@ seginit(void) static pte_t * walkpgdir(pde_t *pgdir, const void *va, int create) { - uint r; pde_t *pde; pte_t *pgtab; pde = &pgdir[PDX(va)]; if(*pde & PTE_P){ - pgtab = (pte_t*) PTE_ADDR(*pde); - } else if(!create || !(r = (uint) kalloc())) - return 0; - else { - pgtab = (pte_t*) r; + pgtab = (pte_t*)PTE_ADDR(*pde); + } else { + if(!create || (pgtab = (pte_t*)kalloc()) == 0) + return 0; // Make sure all those PTE_P bits are zero. memset(pgtab, 0, PGSIZE); // The permissions here are overly generous, but they can // be further restricted by the permissions in the page table // entries, if necessary. - *pde = PADDR(r) | PTE_P | PTE_W | PTE_U; + *pde = PADDR(pgtab) | PTE_P | PTE_W | PTE_U; } return &pgtab[PTX(va)]; } @@ -69,13 +67,16 @@ walkpgdir(pde_t *pgdir, const void *va, int create) static int mappages(pde_t *pgdir, void *la, uint size, uint pa, int perm) { - char *a = PGROUNDDOWN(la); - char *last = PGROUNDDOWN(la + size - 1); + char *a, *last; + pte_t *pte; + + a = PGROUNDDOWN(la); + last = PGROUNDDOWN(la + size - 1); - while(1){ - pte_t *pte = walkpgdir(pgdir, a, 1); + for(;;){ + pte = walkpgdir(pgdir, a, 1); if(pte == 0) - return 0; + return -1; if(*pte & PTE_P) panic("remap"); *pte = pa | perm | PTE_P; @@ -84,7 +85,7 @@ mappages(pde_t *pgdir, void *la, uint size, uint pa, int perm) a += PGSIZE; pa += PGSIZE; } - return 1; + return 0; } // The mappings from logical to linear are one to one (i.e., @@ -122,23 +123,26 @@ kvmalloc(void) pde_t* setupkvm(void) { - pde_t *pgdir; extern char etext[]; - char *rwstart = PGROUNDDOWN(etext); - uint rwlen = (uint)rwstart - 0x100000; + char *rwstart; + pde_t *pgdir; + uint rwlen; + + rwstart = PGROUNDDOWN(etext); + rwlen = (uint)rwstart - 0x100000; // Allocate page directory - if(!(pgdir = (pde_t *) kalloc())) + if((pgdir = (pde_t*)kalloc()) == 0) return 0; memset(pgdir, 0, PGSIZE); if(// Map IO space from 640K to 1Mbyte - !mappages(pgdir, (void *)USERTOP, 0x60000, USERTOP, PTE_W) || + mappages(pgdir, (void*)USERTOP, 0x60000, USERTOP, PTE_W) < 0 || // Map kernel instructions - !mappages(pgdir, (void *)0x100000, rwlen, 0x100000, 0) || + mappages(pgdir, (void*)0x100000, rwlen, 0x100000, 0) < 0 || // Map kernel data and free memory pool - !mappages(pgdir, rwstart, PHYSTOP-(uint)rwstart, (uint)rwstart, PTE_W) || + mappages(pgdir, rwstart, PHYSTOP-(uint)rwstart, (uint)rwstart, PTE_W) < 0 || // Map devices such as ioapic, lapic, ... - !mappages(pgdir, (void *)0xFE000000, 0x2000000, 0xFE000000, PTE_W)) + mappages(pgdir, (void*)0xFE000000, 0x2000000, 0xFE000000, PTE_W) < 0) return 0; return pgdir; } @@ -189,14 +193,15 @@ switchuvm(struct proc *p) // processes directly. char* uva2ka(pde_t *pgdir, char *uva) -{ - pte_t *pte = walkpgdir(pgdir, uva, 0); +{ + pte_t *pte; + + pte = walkpgdir(pgdir, uva, 0); if((*pte & PTE_P) == 0) return 0; if((*pte & PTE_U) == 0) return 0; - uint pa = PTE_ADDR(*pte); - return (char *)pa; + return (char*)PTE_ADDR(*pte); } // Load the initcode into address 0 of pgdir. @@ -204,9 +209,11 @@ uva2ka(pde_t *pgdir, char *uva) void inituvm(pde_t *pgdir, char *init, uint sz) { - char *mem = kalloc(); - if (sz >= PGSIZE) + char *mem; + + if(sz >= PGSIZE) panic("inituvm: more than a page"); + mem = kalloc(); memset(mem, 0, PGSIZE); mappages(pgdir, 0, PGSIZE, PADDR(mem), PTE_W|PTE_U); memmove(mem, init, sz); @@ -223,15 +230,17 @@ loaduvm(pde_t *pgdir, char *addr, struct inode *ip, uint offset, uint sz) if((uint)addr % PGSIZE != 0) panic("loaduvm: addr must be page aligned\n"); for(i = 0; i < sz; i += PGSIZE){ - if(!(pte = walkpgdir(pgdir, addr+i, 0))) + if((pte = walkpgdir(pgdir, addr+i, 0)) == 0) panic("loaduvm: address should exist\n"); pa = PTE_ADDR(*pte); - if(sz - i < PGSIZE) n = sz - i; - else n = PGSIZE; - if(readi(ip, (char *)pa, offset+i, n) != n) - return 0; + if(sz - i < PGSIZE) + n = sz - i; + else + n = PGSIZE; + if(readi(ip, (char*)pa, offset+i, n) != n) + return -1; } - return 1; + return 0; } // Allocate memory to the process to bring its size from oldsz to @@ -241,12 +250,17 @@ loaduvm(pde_t *pgdir, char *addr, struct inode *ip, uint offset, uint sz) int allocuvm(pde_t *pgdir, uint oldsz, uint newsz) { + char *a, *last, *mem; + if(newsz > USERTOP) return 0; - char *a = (char *)PGROUNDUP(oldsz); - char *last = PGROUNDDOWN(newsz - 1); - for (; a <= last; a += PGSIZE){ - char *mem = kalloc(); + if(newsz < oldsz) + return oldsz; + + a = (char*)PGROUNDUP(oldsz); + last = PGROUNDDOWN(newsz - 1); + for(; a <= last; a += PGSIZE){ + mem = kalloc(); if(mem == 0){ cprintf("allocuvm out of memory\n"); deallocuvm(pgdir, newsz, oldsz); @@ -255,7 +269,7 @@ allocuvm(pde_t *pgdir, uint oldsz, uint newsz) memset(mem, 0, PGSIZE); mappages(pgdir, a, PGSIZE, PADDR(mem), PTE_W|PTE_U); } - return newsz > oldsz ? newsz : oldsz; + return newsz; } // Deallocate user pages to bring the process size from oldsz to @@ -265,19 +279,26 @@ allocuvm(pde_t *pgdir, uint oldsz, uint newsz) int deallocuvm(pde_t *pgdir, uint oldsz, uint newsz) { - char *a = (char *)PGROUNDUP(newsz); - char *last = PGROUNDDOWN(oldsz - 1); + char *a, *last; + pte_t *pte; + uint pa; + + if(newsz >= oldsz) + return oldsz; + + a = (char*)PGROUNDUP(newsz); + last = PGROUNDDOWN(oldsz - 1); for(; a <= last; a += PGSIZE){ - pte_t *pte = walkpgdir(pgdir, a, 0); + pte = walkpgdir(pgdir, a, 0); if(pte && (*pte & PTE_P) != 0){ - uint pa = PTE_ADDR(*pte); + pa = PTE_ADDR(*pte); if(pa == 0) panic("kfree"); - kfree((void *) pa); + kfree((char*)pa); *pte = 0; } } - return newsz < oldsz ? newsz : oldsz; + return newsz; } // Free a page table and all the physical memory pages @@ -287,14 +308,14 @@ freevm(pde_t *pgdir) { uint i; - if(!pgdir) + if(pgdir == 0) panic("freevm: no pgdir"); deallocuvm(pgdir, USERTOP, 0); for(i = 0; i < NPDENTRIES; i++){ if(pgdir[i] & PTE_P) - kfree((void *) PTE_ADDR(pgdir[i])); + kfree((char*)PTE_ADDR(pgdir[i])); } - kfree((void *) pgdir); + kfree((char*)pgdir); } // Given a parent process's page table, create a copy @@ -302,22 +323,23 @@ freevm(pde_t *pgdir) pde_t* copyuvm(pde_t *pgdir, uint sz) { - pde_t *d = setupkvm(); + pde_t *d; pte_t *pte; uint pa, i; char *mem; - if(!d) return 0; + if((d = setupkvm()) == 0) + return 0; for(i = 0; i < sz; i += PGSIZE){ - if(!(pte = walkpgdir(pgdir, (void *)i, 0))) + if((pte = walkpgdir(pgdir, (void*)i, 0)) == 0) panic("copyuvm: pte should exist\n"); if(!(*pte & PTE_P)) panic("copyuvm: page not present\n"); pa = PTE_ADDR(*pte); - if(!(mem = kalloc())) + if((mem = kalloc()) == 0) goto bad; - memmove(mem, (char *)pa, PGSIZE); - if(!mappages(d, (void *)i, PGSIZE, PADDR(mem), PTE_W|PTE_U)) + memmove(mem, (char*)pa, PGSIZE); + if(mappages(d, (void*)i, PGSIZE, PADDR(mem), PTE_W|PTE_U) < 0) goto bad; } return d; @@ -334,13 +356,16 @@ bad: int copyout(pde_t *pgdir, uint va, void *xbuf, uint len) { - char *buf = (char *) xbuf; + char *buf, *pa0; + uint n, va0; + + buf = (char*)xbuf; while(len > 0){ - uint va0 = (uint)PGROUNDDOWN(va); - char *pa0 = uva2ka(pgdir, (char*) va0); + va0 = (uint)PGROUNDDOWN(va); + pa0 = uva2ka(pgdir, (char*)va0); if(pa0 == 0) - return 0; - uint n = PGSIZE - (va - va0); + return -1; + n = PGSIZE - (va - va0); if(n > len) n = len; memmove(pa0 + (va - va0), buf, n); @@ -348,5 +373,5 @@ copyout(pde_t *pgdir, uint va, void *xbuf, uint len) buf += n; va = va0 + PGSIZE; } - return 1; + return 0; }