Fix all suggestions from asvitkine

Signed-off-by: Ricky Zhang <rickyzhang@gmail.com>

BasiliskII/src/uae_cpu/newcpu.h

@@ -203,7 +203,7 @@ static __inline__ void m68k_setpc (uaecptr newpc)
 #endif
 
 #if ENABLE_MON
-	if(isBreakPoint(newpc)) {
+	if(IS_BREAK_POINT(newpc)) {
 		printf("Stopped at break point address: %08lx. Last PC: %08lx\n", newpc, previous_pc);
 		m68k_dumpstate(NULL);
 		char *arg[4] = {"mon", "-m", "-r", NULL};
@@ -220,7 +220,7 @@ static __inline__ void m68k_incpc (uae_s32 delta)
 	regs.pc_p += (delta);
 #if ENABLE_MON
 	uaecptr next_pc = m68k_getpc();
-	if(isBreakPoint(next_pc)) {
+	if(IS_BREAK_POINT(next_pc)) {
 		printf("Stopped at break point address: %08lx. Last PC: %08lx\n", next_pc, previous_pc);
 		m68k_dumpstate(NULL);
 		char *arg[4] = {"mon", "-m", "-r", NULL};

cxmon/src/mon.h

@@ -103,6 +103,6 @@ extern uint32 mon_read_word(uintptr adr);
 extern void mon_write_word(uintptr adr, uint32 l);
 
 // Check if break point is set
-#define isBreakPoint(address) (active_break_points.find(address) != active_break_points.end())
+#define IS_BREAK_POINT(address) (active_break_points.find(address) != active_break_points.end())
 
 #endif

cxmon/src/mon_cmd.cpp

@@ -313,9 +313,8 @@ void break_point_add(void)
 {
 	uintptr address;
 
-	if (mon_token == T_END ||
+	if (mon_token == T_END || !mon_expression(&address)) {
-			!mon_expression(&address)) {
+		mon_error("Expect break point in hexadecimal.");
-		fprintf(monerr, "Expect break point in hexadecimal.\n");
 		return;
 	}
 
@@ -335,6 +334,27 @@ void break_point_add(void)
 }
 
 
+bool validate_index(uintptr *index_ptr, const BREAK_POINT_SET& break_point_set)
+{
+	if (mon_token == T_END || !mon_expression(index_ptr)) {
+		mon_error("Expect index number of break point in hexadecimal.\n");
+		return false;
+	}
+
+	if (mon_token != T_END) {
+		mon_error("Too many arguments");
+		return false;
+	}
+
+	if (*index_ptr > break_point_set.size()) {
+		mon_error("Illegal Index Number!");
+		return false;
+	}
+
+	return true;
+}
+
+
 /*
  * Remove Break Point
  */
@@ -342,23 +362,8 @@ void break_point_remove(void)
 {
 	uintptr index;
 
-	if (mon_token == T_END ||
+	if (!validate_index(&index, active_break_points))
-			!mon_expression(&index)) {
-		fprintf(monerr, "Expect index number of break point in hexadecimal.\n");
 		return;
-	}
-
-	if (mon_token != T_END) {
-		mon_error("Too many arguments");
-		return;
-	}
-
-	if (index > active_break_points.size()) {
-		mon_error("Illegal Index Number!");
-		return;
-	}
-
-	BREAK_POINT_SET::iterator it;
 
 	if (0 == index) {
 		active_break_points.clear();
@@ -366,10 +371,8 @@ void break_point_remove(void)
 		return;
 	}
 
-	int pos = 1;
+	BREAK_POINT_SET::iterator it = active_break_points.begin();
-	for (it = active_break_points.begin(); it != active_break_points.end(); it++)
+	std::advance(it, index - 1);
-		if (pos++ == index)
-			break;
 	// Remove break point
 	printf("Removed break point %4x at address %08lx\n", index, *it);
 	active_break_points.erase(it);
@@ -383,36 +386,19 @@ void break_point_disable(void)
 {
 	uintptr index;
 
-	if (mon_token == T_END ||
+	if(!validate_index(&index, active_break_points))
-			!mon_expression(&index)) {
-		fprintf(monerr, "Expect index number of break point in hexadecimal.\n");
 		return;
-	}
-
-	if (mon_token != T_END) {
-		mon_error("Too many arguments");
-		return;
-	}
-
-	if (index > active_break_points.size()) {
-		mon_error("Illegal Index Number!");
-		return;
-	}
-
-	BREAK_POINT_SET::iterator it;
 
 	if (0 == index) {
-		for (it = active_break_points.begin(); it != active_break_points.end(); it++)
+		for (BREAK_POINT_SET::iterator it = active_break_points.begin(); it != active_break_points.end(); it++)
 			disabled_break_points.insert(*it);
 		active_break_points.clear();
 		printf("Disabled all break points!\n");
 		return;
 	}
 
-	int pos = 1;
+	BREAK_POINT_SET::iterator it = active_break_points.begin();
-	for (it = active_break_points.begin(); it != active_break_points.end(); it++)
+	std::advance(it, index - 1);
-		if (pos++ == index)
-			break;
 	// Add to disable break points
 	printf("Disabled break point %4x at address %08lx\n", index, *it);
 	disabled_break_points.insert(*it);
@@ -428,36 +414,19 @@ void break_point_enable(void)
 {
 	uintptr index;
 
-	if (mon_token == T_END ||
+	if(!validate_index(&index, disabled_break_points))
-			!mon_expression(&index)) {
-		fprintf(monerr, "Expect index number of break point in hexadecimal.\n");
 		return;
-	}
-
-	if (mon_token != T_END) {
-		mon_error("Too many arguments");
-		return;
-	}
-
-	if (index > disabled_break_points.size()) {
-		mon_error("Illegal Index Number!");
-		return;
-	}
-
-	BREAK_POINT_SET::iterator it;
 
 	if (0 == index) {
-		for (it = disabled_break_points.begin(); it != disabled_break_points.end(); it++)
+		for (BREAK_POINT_SET::iterator it = disabled_break_points.begin(); it != disabled_break_points.end(); it++)
 			active_break_points.insert(*it);
 		disabled_break_points.clear();
 		printf("Enabled all break points!\n");
 		return;
 	}
 
-	int pos = 1;
+	BREAK_POINT_SET::iterator it = disabled_break_points.begin();
-	for (it = disabled_break_points.begin(); it != disabled_break_points.end(); it++)
+	std::advance(it, index - 1);
-		if (pos++ == index)
-			break;
 	// Add to active break points
 	printf("Disabled break point %4x at address %08lx\n", index, *it);
 	active_break_points.insert(*it);
@@ -477,10 +446,9 @@ void break_point_info(void)
 	}
 
 	BREAK_POINT_SET::iterator it;
-	int pos;
 
 	if (!active_break_points.empty()) {
-		pos = 1;
+		int pos = 1;
 		printf(STR_ACTIVE_BREAK_POINTS);
 		for (it = active_break_points.begin(); it != active_break_points.end(); it++)
 			printf("\tBreak point %4x at address %08lx\n", pos++, *it);
@@ -489,7 +457,7 @@ void break_point_info(void)
 	if (!disabled_break_points.empty()) {
 		putchar('\n');
 		printf(STR_DISABLED_BREAK_POINTS);
-		pos = 1;
+		int pos = 1;
 		for (it = disabled_break_points.begin(); it != disabled_break_points.end(); it++)
 			printf("\tBreak point %4x at address %08lx\n", pos++, *it);
 	}
@@ -560,7 +528,7 @@ void break_point_load(void)
 		mon_error("Unable to create file");
 	else{
 		char line_buff[1024];
-		bool isDisabledBreakPoints = false;;
+		bool is_disabled_break_points = false;;
 
 		if(fgets(line_buff, sizeof(line_buff), file) == NULL ||
 				strcmp(line_buff, STR_ACTIVE_BREAK_POINTS) != 0) {
@@ -570,14 +538,14 @@ void break_point_load(void)
 
 		while(fgets(line_buff, sizeof(line_buff), file) != NULL) {
 			if(strcmp(line_buff, STR_DISABLED_BREAK_POINTS) == 0) {
-				isDisabledBreakPoints = true;
+				is_disabled_break_points = true;
 				continue;
 			}
 			uintptr address;
 			std::stringstream ss;
 			ss << std::hex << line_buff;
 			ss >> address;
-			if(isDisabledBreakPoints)
+			if(is_disabled_break_points)
 				disabled_break_points.insert(address);
 			else
 				active_break_points.insert(address);

cxmon/utils/README.md

@@ -1,8 +1,8 @@
 # What
-suspend.bin is a MacBinary file which supposes to upack and run in M68k Macintosh only. It runs emul_op `0x7138` and trigger BasiliskII into cxmon so that you can add break points there.
+suspend.bin is a MacBinary file which should be upacked and run in M68k Macintosh only. It runs emul_op `0x7138` and trigger BasiliskII into cxmon so that you can add break points there.
 
 # How
-1. You must build Basilisk II `--with-mon` options.
+1. You must build Basilisk II `--with-mon=YES` options.
 1. Copy suspend.bin into Macintosh guest OS.
 1. Unpack it with MacBinary.
 1. Run the program when you want to add break points.