pintos-os.org Git - pintos-anon/blob - grading/userprog/review.txt

   1 Test cases [[/25]]
   2 ------------------
   3   -15 Didn't write own test cases
   4   -10 Insufficient testing
   5
   6 Design [[/100]]
   7 ---------------
   8
   9 Quality of DESIGNDOC
  10   -10 Arg passing
  11   -20 Copying data around:
  12       User-to-kernel copying.
  13       Kernel-to-user copying.
  14       String copying.
  15   -20 System calls:
  16       Allocation of file descriptors.
  17       Handling exceptions and related cleanup.
  18       pid_t rationale (if they changed tid_t -> pid_t mapping).
  19       Synchronization of system calls and filesystem.
  20
  21 Overall:
  22   -1 Gratuitous use of malloc() (e.g. for allocating a list or a lock)
  23   -1 Inappropriate use of ASSERT (e.g. to verify that malloc() succeeded)
  24
  25 Program arguments:
  26   +1 Support multiple pages of arguments.
  27
  28 User/kernel copying:
  29   -5 Too many copies of user/kernel copying code
  30   -20 Doesn't check for page boundaries
  31   -10 Imperfect checking for page boundaries
  32   -5 Doesn't check whether pointers are at or above PHYS_BASE
  33   -2 Imperfect checking whether pointers are at or above PHYS_BASE
  34   +3 Copies large chunks while properly observing page boundaries
  35   +3 Scans for string null terminators w/o checking individual bytes
  36      while properly observing page boundaries
  37   +3 Uses get_user() and put_user() functions from FAQ for copying
  38
  39 System call design:
  40   -5 Disables interrupts without reasonable justification
  41   -2 Doesn't close open files at process exit
  42   -2 Doesn't acquire file system lock to close files at process exit
  43   -5 Buffer overflow in read or write system call
  44   -5 System call error exit leaks memory/fails to release global lock
  45   -5 Uses a pointer as a file descriptor or pid without justifying
  46
  47 Wait system call:
  48   -3 Busy waiting
  49   -3 A static list of all parent-child pairs is extremely wasteful
  50   -3 Obviously wasteful with memory (not deleting processes)
  51   -2 Finished parent deletes children which may still be running
  52   -1 Enable/disable interrupts
  53   -2 Joinable child lets its struct thread be deleted before parent dies
  54   -1 Race condition between wait and thread exit
  55
  56 Style [[/25]]
  57 -------------
  58   -5 Extraneous output caused warnings
  59   -5 Didn't print process termination messages
  60   -5 One big function for handling system calls
  61   -5 No attempt to conform to existing coding style
  62
  63 Comments
  64 --------