X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=blobdiff_plain;f=grading%2Fuserprog%2Freview.txt;h=c48657e1898666fd1d3a805bcd8248be1cb9560a;hb=8f541a94d9e93c6040a38587ffafdc22f06eca0e;hp=38ea7ac91f3389681d0ddfc8ba819705b11e62ed;hpb=fe106fae62a28fbcaec46830859884b98f577159;p=pintos-anon

diff --git a/grading/userprog/review.txt b/grading/userprog/review.txt
index 38ea7ac..c48657e 100644
--- a/grading/userprog/review.txt
+++ b/grading/userprog/review.txt
@@ -1,65 +1,55 @@
-Design (/120 points)
---------------------
+Test cases [[/25]]
+------------------
+  -15 Didn't write own test cases
+  -10 Insufficient testing
 
-a) DESIGNDOC (/60 pts)
-    a) Arg passing                                          (/10)
-    b) System calls                                         (/20)
-         Didn't discuss global vs local allocation of fileID's
-         How to handle exceptions & why (ie killing processes
-           that do bad things)
-         Explaining choice of openFileID's & ASIDS
-    c) Multiprogramming                                     (/30)
-         How to find a free page.
-         Explaining how new processes are executed.
-         How they deal with running out of memory error on loading.
+Design [[/100]]
+---------------
 
-b) Other Design (/60 pts)
+Quality of DESIGNDOC
+  -10 Arg passing
+  -20 Copying data around:
+      User-to-kernel copying.
+      Kernel-to-user copying.
+      String copying.
+  -20 System calls:
+      Allocation of file descriptors.
+      Handling exceptions and related cleanup.
+      pid_t rationale (if they changed tid_t -> pid_t mapping).
+      Synchronization of system calls and filesystem.
 
-   1. Argument passing
-      No checking for page boundaries in argument passing               -10
-      Uses memcpy/bcopy/memmove or file reading/writing for             +5
-        copying up to/starting from page boundaries -- good job!
-        (Search for memcpy/bcopy, not just extern, make sure they call it
-        with the possibility of copying more than 4 bytes at a time. ALSO if
-        they do File Read/Writes then that is not byte-by-byte but they will
-        fail page crossing only take off for failing the pagecrossing test.)
-      Don't decomp / reuse translation code                             -5
-      Using strtok instead of strtok_r or strsep                        -1
-        (strtok is not thread-safe)
+Overall:
+  -1 Gratuitous use of malloc() (e.g. for allocating a list or a lock)
+  -1 Inappropriate use of ASSERT (e.g. to verify that malloc() succeeded)
 
-   2. System calls
-      No checking for page boundaries in system calls                   -20
-        (-5 if they only forgot for some system calls but did it somewhere)
-      Interrupts turned off for all system calls                        -10
-      Doesn't close open files after process exits                      -2
-      table/when file is closed, it is not removed from file table
-      Uses printf/fscanf/putc/getc when interacting                     -5
-        with StandardInput / StandardOutput
-      Using fixed length buffer for Read/Write                          -5
-      Using pointer to int casts as ASID/FileId without justifying      -5
+Program arguments:
+  +1 Support multiple pages of arguments.
 
-   3. Multiprogramming
-      Don't use the bitmap object                                       -5
-      Bitmap is not cleared when process dies                           -5
-      Access to bitmap is not sychronized                               -5
-      getting next ASID is not synchronized                             -5
-        ...or (mututally exclusive, depending on their solution)...
-      Doesn't use ASID's at all, but their system for identifying       -5
-        processes has synchronization or uniqueness problems (e.g. 
-        careless use of AddrSpace*'s)
-      Multiple translations for a page                                  -10
-        (One vaddr maps to two paddr's, or vice versa)
+User/kernel copying:
+  -5 Too many copies of user/kernel copying code
+  -10 Doesn't check for page boundaries
+  -3 Imperfect checking for page boundaries
+  -5 Doesn't check whether pointers are at or above PHYS_BASE
+  -2 Imperfect checking whether pointers are at or above PHYS_BASE
+  +3 Copies large chunks while properly observing page boundaries
+  +3 Scans for string null terminators w/o checking individual bytes
+     while properly observing page boundaries
+  +3 Uses get_user() and put_user() functions from FAQ for copying
 
-Style (/30 points)
-------------------
-0) Had to modify code to get to compile the first                       -10
-   time
-1) Minor changes to machine code                                        -10
-2) Extraneous output                                                    -5
-3) Unexplained dependencies                                             -5
-4) Not Decomposing Exception handler                                    -5
+System call design:
+  -5 Disables interrupts without reasonable justification
+  -2 Doesn't close open files at process exit
+  -2 Doesn't acquire file system lock to close files at process exit
+  -5 Buffer overflow in read or write system call
+  -5 System call error exit leaks memory/fails to release global lock
+  -5 Uses a pointer as a file descriptor or pid without justifying
+
+Style [[/25]]
+-------------
+  -5 Extraneous output caused warnings
+  -5 Didn't print process termination messages
+  -5 One big function for handling system calls
+  -5 No attempt to conform to existing coding style
 
-TESTCASEs (/30 points)
-----------------------
-    Don't write own testcases                                           -15
-    Not sufficient testing                                        up to -10
+Comments
+--------