diff --git a/src/userprog/process.c b/src/userprog/process.c index e1200bc..5c0b356 100644 --- a/src/userprog/process.c +++ b/src/userprog/process.c @@ -49,7 +49,7 @@ struct process_start_data be started. */ bool success; /* Indicates whether the process was successfully loaded. */ struct semaphore loaded; /* Semaphore used to signal that the process has - been loaded. */ + finished attempting to load. */ }; static thread_func start_process NO_RETURN; @@ -64,14 +64,7 @@ process_execute (const char *cmd) { char *cmd_copy; tid_t tid; - - struct process_start_data *data = malloc (sizeof (struct process_start_data)); - if (data == NULL) - { - return TID_ERROR; - } - sema_init (&data->loaded, 0); - data->success = false; + struct process_start_data data; /* Make a copy of command. Otherwise there's a race between the caller and load(). */ @@ -85,13 +78,10 @@ process_execute (const char *cmd) /* Retrieve first argument of command, which is the file name of the process. */ - char *file_name = strtok_r (cmd_copy, " ", &data->cmd_saveptr); + char *file_name = strtok_r (cmd_copy, " ", &data.cmd_saveptr); - /* NOTE: Currently, the file being executed is closed in load () and then - reopened here. Because load is an exported public function, this - might be necessary. */ + /* Validates that the current file to be executed can be opened/exists. */ lock_acquire (&filesys_lock); - /* Validates that the current file to be executed is a valid file */ bool valid_file = filesys_open (file_name) != NULL; lock_release (&filesys_lock); if (!valid_file) @@ -100,19 +90,26 @@ process_execute (const char *cmd) /* Create a new thread to execute the command, by initializing it running the function 'start_process' with the appropriate arguments. For details of arguments, see 'start_process'. */ - data->cmd = cmd_copy; - strlcpy (data->file_name, file_name, FNAME_MAX_LEN + 1); + data.cmd = cmd_copy; + strlcpy (data.file_name, file_name, FNAME_MAX_LEN + 1); + sema_init (&data.loaded, 0); + data.success = false; - tid = thread_create (file_name, PRI_DEFAULT, start_process, data); + tid = thread_create (file_name, PRI_DEFAULT, start_process, &data); + + /* Return TID_ERROR and free resources if starting execution went wrong. */ if (tid == TID_ERROR) palloc_free_page (cmd_copy); else { - sema_down (&data->loaded); - if (!data->success) + + /* Wait until process file has finished attempting to load via the child + thread before reporting success of starting execution. */ + sema_down (&data.loaded); + if (!data.success) tid = TID_ERROR; } - free (data); + return tid; } @@ -124,13 +121,15 @@ static void *push_to_stack (void **esp, void *data, size_t data_size); /* Make the current thread execute 'cmd', passing in a copy of the command string used for processing, the saveptr used by strtok_r (in order to further tokenize the same command and retrieve its - arguments), as well as the name of the file being executed. This - involves loading the specified file and starting it running. */ + arguments), the name of the file being executed, and a semaphore that + calls sema_up to indicate that the 'success' variable passed to it + has been updated to indicate whether the process file loading succeeded. + This involves loading the specified file and calling its main () function + with the specified command arguments. */ static void start_process (void *proc_start_data) { struct intr_frame if_; - bool success; struct process_start_data *data = proc_start_data; @@ -147,34 +146,36 @@ start_process (void *proc_start_data) if (exec_file == NULL) { lock_release (&filesys_lock); - goto fail; + sema_up (&data->loaded); + thread_exit (); } - thread_current ()->exec_file = exec_file; file_deny_write (exec_file); lock_release (&filesys_lock); + thread_current ()->exec_file = exec_file; - success = load (data->file_name, &if_.eip, &if_.esp); + data->success = load (data->file_name, &if_.eip, &if_.esp); /* If load failed, quit. */ - if (!success) + if (!data->success) { palloc_free_page (data->cmd); - goto fail; + sema_up (&data->loaded); + thread_exit (); } /* Initialize user process stack and free page used to store the command that executed the process. */ - success = process_init_stack (data->cmd_saveptr, &if_.esp, data->file_name); + data->success = process_init_stack (data->cmd_saveptr, &if_.esp, data->file_name); + bool success = data->success; palloc_free_page (data->cmd); + sema_up (&data->loaded); /* If stack initialization failed, free resources and quit. */ if (!success) { - goto fail; + thread_exit (); } - data->success = true; - sema_up (&data->loaded); /* Start the user process by simulating a return from an interrupt, implemented by intr_exit (in threads/intr-stubs.S). Because intr_exit takes all of its @@ -183,12 +184,6 @@ start_process (void *proc_start_data) and jump to it. */ asm volatile ("movl %0, %%esp; jmp intr_exit" : : "g" (&if_) : "memory"); NOT_REACHED (); - - /* If starting the process failed, exit. */ -fail: - data->success = false; - sema_up (&data->loaded); - thread_exit (); } /* Helper function that initializes the stack of a newly created