Improved comments, prepped for submission

assignment2_handout.c

@@ -1,6 +1,12 @@
 /* Dempsey-Jensen, Brychan, 14299890, Assignment 2, 159.341 */
 /* Use semaphores to prevent race conditions in a concurrent lift-simulator */
 
+// *************
+// Note:
+// Uncommenting this definition will print populations & the number of people next to the building
+
+// #define POPULATIONS
+
 // --------------------------------------------------
 // ---   159.341 Assignment 2 - Lift Simulator    ---
 // --------------------------------------------------
@@ -25,7 +31,7 @@
 //#define SUPERSLOW
 //#define SLOW
 //#define FAST
-#define SUPERFAST
+#define SUPERFAST // Original FAST
 
 #if defined(SUPERSLOW)
 	#define LIFTSPEED 500	  // The time it takes for the lift to move one floor
@@ -59,8 +65,7 @@
 #if defined(_WIN32) || defined(WIN32)
 #define true TRUE
 #endif
-// Print the number of people in the lifts & on the floors
+
-//#define POPULATIONS
 
 // --------------------------------------------------
 // Information about a floor in the building
@@ -71,7 +76,8 @@ typedef struct
 	int waitingtogodown;		// The number of people waiting to go down
 	semaphore up_arrow;			// People going up wait on this
 	semaphore down_arrow;		// People going down wait on this
-	semaphore queueInteraction; // People can't all press a button at the same time
+	// People can't all press a button at the same time
+	semaphore queueInteraction; 
 	} floor_info;
 
 // --------------------------------------------------
@@ -85,6 +91,7 @@ typedef struct
 	int peopleinlift;			// The number of people in the lift
 	int stops[NFLOORS];			// How many people are going to each floor
 	semaphore stopsem[NFLOORS]; // People in the lift wait on one of these
+	// control whether the lift is currently adding someone
 	semaphore addToStop;
 } lift_info;
 
@@ -92,11 +99,14 @@ typedef struct
 // Some global variables
 // --------------------------------------------------
 floor_info floors[NFLOORS];
-lift_info *targetLift[NFLOORS]; // 2-dimensional array of the lift
+
+// keep track of the current target lift on the floor (and a semaphore to prevent this changing prematurely)
+lift_info *targetLift[NFLOORS];
 semaphore targetLiftSelect[NFLOORS];
 
-// Bound all printing to this semaphore
+// Bind all printing to this semaphore, so that the target position can't be changed before printing.
 semaphore printSemaphore;
+
 // If printing people counts, int array and semaphore to keep track of them
 #ifdef POPULATIONS
 semaphore updateFloorCount;
@@ -105,25 +115,14 @@ int floorCount[NFLOORS];
 int floorCountZeroInc[NFLOORS];
 
 // --------------------------------------------------
-// Print a string on the screen at position (x,y)
+// Prints a string, with a delay - but it is zero ms
 // --------------------------------------------------
 void print_at_xy_fast(int x, int y, const char *s)
 {
-	// Lock our print to just one thread (stops x,y from changing)
 	semaphore_wait(&printSemaphore);
-	// Move cursor to (x,y)
 	gotoxy(x, y);
-
-	// Slow things down
 	Sleep(0);
-
-	// Print the string
 	printf("%s", s);
-
-	// Move cursor out of the way
-	gotoxy(42, NFLOORS + 2);
-	// flush the stream and release the semaphore
-	//fflush(stdout);
 	semaphore_signal(&printSemaphore);
 }
 #endif
@@ -179,9 +178,8 @@ void get_into_lift(lift_info *lift, int direction)
 		waiting = &floors[lift->position].waitingtogodown;
 	}
 
-	// Problems here are primarily state consistency. Between the start of the conditional, the number of waiting people can change, resulting
+	// Waiting can begin to be decremented while another thread enters the loop. Therefore we
-	// in the number of waiting people dropping below zero.
+	// need to prevent an attempt to remove someone who has already been removed
-	// For all the people waiting
 	while (*waiting)
 	{
 		// Check if lift is empty
@@ -191,8 +189,7 @@ void get_into_lift(lift_info *lift, int direction)
 			lift->direction = direction;
 		}
 
-		// Check there are people waiting and lift isn't full. Ignore in-progress (moving into lift) people
+		// Prevent other threads from evaluating the conditional until waiting is decremented
-		// must check here
 		semaphore_wait(&floors[lift->position].queueInteraction);
 		if (lift->peopleinlift < MAXNOINLIFT && *waiting)
 		{
@@ -205,16 +202,20 @@ void get_into_lift(lift_info *lift, int direction)
 
 			// Wait for person to get into lift
 			Sleep(GETINSPEED);
-			// Need a sem here
+
-			// Wait til we can be the lift that is loading
+
+			// Wait to set the current lift for the floor as our current lift. We maintain this lock,
+			// while the person whom is later signalled will relase it
 			semaphore_wait(&targetLiftSelect[lift->position]);
 			targetLift[lift->position] = lift;
-			// lock the addtostop
+			// lock the addtostop, to prevent the lift leaving without the passenger
 			semaphore_wait(&lift->addToStop);
+			// Finally, "open the doors and let someone in"
 			semaphore_signal(s);
 		}
 		else
 		{
+			// There is no-one else waiting; so release our lock
 			semaphore_signal(&floors[lift->position].queueInteraction);
 			break;
 		}
@@ -236,9 +237,8 @@ void *lift_thread(void *p)
 	lift.position = 0;	   // Lift starts on ground floor
 	lift.direction = UP;   // Lift starts going up
 	lift.peopleinlift = 0; // Lift starts empty
-	// lift is ok with loading a single person
+	// A lift can load a single person at a time
 	semaphore_create(&lift.addToStop, 1);
-	//semaphore_create(&lift.buttonPress, 1);
 
 	for (i = 0; i < NFLOORS; i++)
 	{
@@ -271,9 +271,10 @@ void *lift_thread(void *p)
 		// Drop off passengers on this floor
 		while (lift.stops[lift.position] != 0)
 		{
-			// These are sequential, so don't need protecting
+			// These are sequential operations on just one thread (this lift), so don't need protecting here -
-			// One less passenger in lift
+			// The passenger is told to get out and then does work on the floor
 
+			// One less passenger in lift
 			lift.peopleinlift--;
 
 			// One less waiting to get off at this floor
@@ -309,9 +310,8 @@ void *lift_thread(void *p)
 		// Erase lift from screen
 		print_at_xy(no * 4 + 1, NFLOORS - lift.position, (lift.direction + 1 ? " " : lc));
 
-		// Move lift
+		// Move lift, but make sure it doesn't move before someone is fully loaded
-		// Hold the lift while someone is being added, before moving off
+		// this really could go anywhere, but it fits the metaphor to wrap it here (doesn't have to wrap anything)
-		// this really could go anywhere, but it fits the metaphor to wrap it here (doesn't need to wrap anything)
 		semaphore_wait(&lift.addToStop);
 		lift.position += lift.direction;
 		semaphore_signal(&lift.addToStop);
@@ -353,11 +353,12 @@ void *person_thread(void *p)
 		if (to > from)
 		{
 			// One more person waiting to go up
+			// Only one can press the button at a time. Ensure the person is printed to the console before releasing the
+			// semaphore so that there are no ghosts left in the console
 			semaphore_wait(&floors[from].queueInteraction);
 			floors[from].waitingtogoup++;
 			print_at_xy(NLIFTS * 4 + floors[from].waitingtogoup + floors[from].waitingtogodown, NFLOORS - from, pr);
 			semaphore_signal(&floors[from].queueInteraction);
-			// Print person waiting
 
 			s = &floors[from].up_arrow;
 			// Wait for a lift to arrive (going up)
@@ -369,12 +370,11 @@ void *person_thread(void *p)
 			floors[from].waitingtogodown++;
 			print_at_xy(NLIFTS * 4 + floors[from].waitingtogoup + floors[from].waitingtogodown, NFLOORS - from, pr);
 			semaphore_signal(&floors[from].queueInteraction);
-			// Print person waiting
-			// We want this to happen after the semaphore is released, so the person is added after any deletions
 
 			s = &floors[from].down_arrow;
 			// Wait for a lift to arrive (going down)
 		}
+
 		// We've pressed our button
 		// waiting for a lift's doors to open
 		semaphore_wait(s);
@@ -382,6 +382,7 @@ void *person_thread(void *p)
 		lift = targetLift[from];
 		// Realease the targetLift ASAP so another lift can load someone
 		semaphore_signal(&targetLiftSelect[lift->position]);
+
 		// GetIntoLift() continues its loop, from here (this passenger is signalled to enter; so we definitely need to hold a lock around
 		// this value)
 		// When GetIntoLift() decides that a passenger should enter, it obtains a lock of addToStop. 
@@ -390,21 +391,20 @@ void *person_thread(void *p)
 
 		// We could do this by extending the targetLiftSelect semaphore through the same condition, but that would affect the performance of all lifts on this floor
 		lift->stops[to]++;
-		semaphore_signal(&lift->addToStop);
+		
-		// Press button if we are the first
-		// Technically, the semaphore should extend through this conditional, but as it is only a read operation whose state is should to
-		// be greater than 0 here, it doesn't significantly matter to overwrite the char again (the performance impact is low)
 		if (lift->stops[to] == 1)
 		{
 			// Print light for destination
 			print_at_xy(lift->no * 4 + 1 + 2, NFLOORS - to, "-");
 		}
+		// Person are fully added
+		semaphore_signal(&lift->addToStop);
 
 		semaphore_wait(&lift->stopsem[to]);
 		// Wait until we are at the right floor
 
-
 #ifdef POPULATIONS
+		// Prints the current location of all people in a line below the building
 		semaphore_wait(&updateFloorCount);
 		floorCount[to]++;
 		floorCount[from]--;
@@ -415,7 +415,7 @@ void *person_thread(void *p)
 		print_at_xy_fast((long long)p * 4 + 1, NFLOORS + 8, "  ");
 		print_at_xy_fast((long long)p * 4 + 1, NFLOORS + 8, str);
 #endif
-		// Exit the lift
+		// Finally, exit the lift
 		from = to;
 	}
 
@@ -489,6 +489,7 @@ void *update_floor_counts(void *p)
 {
 	int i;
 	int k;
+	// Iterations since the count on the floor was zero
 	char str2[18] = {';',' ','\t', 'i', 't', 'e', 'r', 's', ' ', 's', 'i', 'n', 'c', 'e', ' ', '0', ':', ' '};
 	while (true)
 	{
@@ -546,11 +547,11 @@ int main()
 	char c;
 	do {
     	c = getchar();
-    	//putchar (c);
+  	} while (!(c == 10 || c == 12)); // Char is not \r or \n
-  	} while (!(c == 10 || c == 12));
 	
 	// Local variables
 	unsigned long i;
+	// init the print semaphore
 	semaphore_create(&printSemaphore, 0);
 	// Initialise Building
 	for (i = 0; i < NFLOORS; i++)
@@ -559,12 +560,13 @@ int main()
 		floors[i].waitingtogodown = 0;
 		semaphore_create(&floors[i].up_arrow, 0);
 		semaphore_create(&floors[i].down_arrow, 0);
-		semaphore_create(&floors[i].queueInteraction, 1); // This initially has one available button press
+
-		semaphore_create(&targetLiftSelect[i], 1); // This initially has one available button press
+		semaphore_create(&floors[i].queueInteraction, 1);
+		semaphore_create(&targetLiftSelect[i], 1);
 #ifdef POPULATIONS
 		if (i == 0)
 		{
-			semaphore_create(&updateFloorCount, 1); // Moved, to save excessive preprocessor tags
+			semaphore_create(&updateFloorCount, 1);
 			floorCount[0] = NPEOPLE;
 			floorCountZeroInc[0] = 0;
 		}
@@ -574,8 +576,8 @@ int main()
 			floorCountZeroInc[i] = 0;
 		}
 #endif
+
 	}
-	// --- Initialise any other semaphores ---
 
 	// Print Building
 	printbuilding();