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.

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<elf.phnum; i++, off+=sizeof(ph)){
     if(readi(ip, (char*)&ph, off, sizeof(ph)) != sizeof(ph))
       goto bad;
@@ -38,9 +40,9 @@ exec(char *path, char **argv)
       continue;
     if(ph.memsz < ph.filesz)
       goto bad;
-    if(!(sz = allocuvm(pgdir, sz, ph.va + ph.memsz)))
+    if((sz = allocuvm(pgdir, sz, ph.va + ph.memsz)) == 0)
       goto bad;
-    if(!loaduvm(pgdir, (char *)ph.va, ip, ph.offset, ph.filesz))
+    if(loaduvm(pgdir, (char*)ph.va, ip, ph.offset, ph.filesz) < 0)
       goto bad;
   }
   iunlockput(ip);
@@ -48,7 +50,7 @@ exec(char *path, char **argv)
 
   // Allocate a one-page stack at the next page boundary
   sz = PGROUNDUP(sz);
-  if(!(sz = allocuvm(pgdir, sz, sz + PGSIZE)))
+  if((sz = allocuvm(pgdir, sz, sz + PGSIZE)) == 0)
     goto bad;
 
   // initialize stack content:
@@ -64,24 +66,22 @@ exec(char *path, char **argv)
   // argc                             -- argc argument to main()
   // ffffffff                         -- return PC for main() call
 
-  uint sp = sz;
+  sp = sz;
 
   // count arguments
-  int argc;
   for(argc = 0; argv[argc]; argc++)
     ;
   if(argc >= 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;
 }