From e63c6a8bad9a98baee24d2d4ef1e1d9035ce265b Mon Sep 17 00:00:00 2001
From: manuel <manuel@mausz.at>
Date: Tue, 27 Mar 2012 20:22:10 +0200
Subject: cast esp to uint32_t before setting up the stack. this avoids a few
 casts but also creates a few casts. overall I think it's more sane this way.

---
 userprog/process.c | 64 +++++++++++++++++++++++++++---------------------------
 1 file changed, 32 insertions(+), 32 deletions(-)

(limited to 'userprog')

diff --git a/userprog/process.c b/userprog/process.c
index bc8a1a9..fdfb5dd 100644
--- a/userprog/process.c
+++ b/userprog/process.c
@@ -33,7 +33,7 @@ struct lock filesys_lock;
 /* prototypes */
 static thread_func start_process NO_RETURN;
 static bool load (const char *args, void (**eip) (void), void **esp);
-static bool setup_stack (void **esp, const char *args);
+static bool setup_stack (uint32_t **esp, const char *args);
 static bool init_fd_table (struct fd_table * table);
 
 /* Initialize the filesystem lock */
@@ -457,7 +457,7 @@ load (const char *args, void (**eip) (void), void **esp)
   }
 
   /* Set up stack. */
-  if (!setup_stack (esp, args))
+  if (!setup_stack ((uint32_t **) esp, args))
     goto done;
 
   /* Start address. */
@@ -589,13 +589,13 @@ load_segment (struct file *file, off_t ofs, uint8_t *upage,
    You will implement this function in the Project 0.
    Consider using `hex_dump` for debugging purposes */
 static bool
-setup_stack (void **esp, const char *args)
+setup_stack (uint32_t **esp, const char *args)
 {
   uint8_t *kpage = NULL;
   const char *name = thread_current ()->name;
-  uint8_t *argv_cur, *argv_end = PHYS_BASE;
-  int argc = 0;
-  unsigned argslen = strlen(args);
+  char *argv_cur;
+  uint32_t argc = 0;
+  unsigned namelen, argslen = strlen(args);
 
   kpage = palloc_get_page (PAL_USER | PAL_ZERO);
   if (kpage == NULL)
@@ -612,30 +612,30 @@ setup_stack (void **esp, const char *args)
   if (argslen > 0)
     {
       argslen += 1; /* add the trailing \0 */
-      *esp -= argslen;
+      *(char **) esp -= argslen;
       memcpy(*esp, args, argslen);
-      argv_end = *esp;
     }
 
   /* copy executable name to stack */
-  *esp -= strlen(name) + 1;
-  memcpy(*esp, name, strlen(name) + 1);
+  namelen = strlen(name) + 1;
+  *(char **) esp -= namelen;
+  memcpy(*esp, name, namelen);
 
-  /* align our stack so far by word-size */
-  *esp -= (sizeof(uint32_t) - (PHYS_BASE - *esp) % sizeof(uint32_t)); /* thanks to thomas & edy */
+  /* align our currend address by word-size (thanks to thomas & edy) */
+  *(char **) esp -= (sizeof(uint32_t) - (PHYS_BASE - *(void **) esp) % sizeof(uint32_t));
 
   /* terminate argv[] array by NULL ptr */
-  *esp -= sizeof(uint32_t);
-  *(uint32_t *) *esp = '\0';
+  (*esp)--;
+  **esp = '\0';
 
   /* push the argv[] entries */
   if (argslen > 0)
     {
       /* we walk through our already pushed arguments in reverse order and
-         replace every occurrence of the space-character with '\0'-character
-         due to the fact we we walk in reverse order we can easily populate
-         the pointers to the elements of argv[] too */
-      for(argv_cur = PHYS_BASE - 1; argv_cur >= argv_end; argv_cur--)
+         replace every occurrence of the space-character with '\0'-character.
+         due to the fact we walk in reverse order we can easily push
+         the pointers to the elements of argv[] too. */
+      for(argv_cur = PHYS_BASE - 1; argv_cur >= (char *)PHYS_BASE - argslen; argv_cur--)
         {
           /* check for space-character and replace if necessary */
           if (*argv_cur == ' ')
@@ -644,12 +644,12 @@ setup_stack (void **esp, const char *args)
 
               /* since we walk in reverse order and already have replaced a
                  character we simply check the preceding character. if it's
-                 a non-'\0'-character we know an argument starts here */
+                 a non-'\0'-character we know an argument starts here. */
               if (*(argv_cur + 1) != '\0')
                 {
                   /* push pointer to argument */
-                  *esp -= sizeof(uint32_t);
-                  *(uint32_t *) *esp = (uint32_t) argv_cur + 1;
+                  (*esp)--;
+                  **esp = (uint32_t) argv_cur + 1;
                   argc++;
                 }
             }
@@ -659,28 +659,28 @@ setup_stack (void **esp, const char *args)
          end of the loop. so let's do that */
       if (*(argv_cur + 1) != '\0')
         {
-          *esp -= sizeof(uint32_t);
-          *(uint32_t *) *esp = (uint32_t) argv_cur + 1;
+          (*esp)--;
+          **esp = (uint32_t) argv_cur + 1;
           argc++;
         }
     }
 
   /* push argv[0] (executable name) */
-  *esp -= sizeof(uint32_t);
-  *(uint32_t *) *esp = (uint32_t) (PHYS_BASE - (argslen + strlen(name) + 1));
+  (*esp)--;
+  **esp = (uint32_t) (PHYS_BASE - (argslen + namelen));
   argc++;
 
-  /* push pointer to argv[]. this is simple as it's our current address */
-  *(uint32_t *) (*esp - 4) = *(uint32_t *) esp;
-  *esp -= sizeof(uint32_t);
+  /* push pointer to argv[]. this is simple as it's our last address */
+  (*esp)--;
+  **esp = (uint32_t) (*esp + 1);
 
   /* push argc */
-  *esp -= sizeof(uint32_t);
-  *(uint32_t *) *esp = argc;
+  (*esp)--;
+  **esp = (uint32_t) argc;
 
   /* push fake return address */
-  *esp -= sizeof(uint32_t);
-  *(uint32_t *) *esp = 0;
+  (*esp)--;
+  **esp = 0;
 
   return true;
 }
-- 
cgit v1.2.3