cmd: report a rejected command as an error and keep it out of the history

run_cmd_body() answers POST /cmd with 400 when err_status is set, but
only the tokenizer set it. Every "Error:", "Invalid ..." and "Unknown
command" from the parsers went back as 200, so the web page reported the
command as applied, and save_cmd put the rejected line into the command
history, from where Save to flash wrote it into the startup config.

Set ERR_INVALID_ARGUMENT on every path that rejects a command: the err:
labels of the parsers, the parsers that print their own error, the
stp parser and the unknown command fallback. Warnings that still apply
the command, such as a full VLAN name table, are left alone.

Record a command in the history only when it was accepted, and let
execute_commands() report the first failure of a multi-line body, since
cmd_tokenize() clears err_status again for every following line.

cmd_parser.c

@@ -441,10 +441,12 @@ void parse_mac_cmd(void)
 		return;
 	}
 	if (cmd_words_len != 2 || !parse_mac(cmd_words_b[1])) {
+		err_status = ERR_INVALID_ARGUMENT;
 		print_string("Error: mac [<aa:bb:cc:dd:ee:ff>]\n");
 		return;
 	}
 	if ((mac_parse_result[0] & 0x03) || ((mac_parse_result[0] | mac_parse_result[1] | mac_parse_result[2]) == 0)) {
+		err_status = ERR_INVALID_ARGUMENT;
 		print_string("refusing: must be unicast, globally administered\n");
 		return;
 	}
@@ -523,6 +525,7 @@ void parse_lag(void)
 	port_lag_members_set(group, members);
 	return;
 err:
+	err_status = ERR_INVALID_ARGUMENT;
 	print_string("Error: lag (show | <1-4> (d | <port>...))\n");
 }
 
@@ -565,6 +568,7 @@ void parse_lag_hash(void)
 	port_lag_hash_set(group, hash);
 	return;
 err:
+	err_status = ERR_INVALID_ARGUMENT;
 	print_string("Error: laghash <1-4> [smac|dmac|sip|dip|sport|dport]\n");
 }
 
@@ -663,6 +667,7 @@ void parse_vlan(void)
 	}
 	return;
 err:
+	err_status = ERR_INVALID_ARGUMENT;
 	print_string("Error: vlan (<vlan-id>|show) [port][t]...\n");
 }
 
@@ -714,6 +719,7 @@ void parse_isolate(void)
 	return;
 
 err:
+	err_status = ERR_INVALID_ARGUMENT;
 	print_string("Error: isolate <port> [show|off] [port]...\n");
 }
 
@@ -751,6 +757,7 @@ void parse_ingress(void)
 		// Setting mode for all ports at once
 		for (log_port = machine.min_port; log_port <= machine.max_port; log_port++) {
 			if (!port_ingress_filter(log_port, mode)) {
+				err_status = ERR_INVALID_ARGUMENT;
 				print_string("Error setting ingress filter for port "); print_phys_port(log_port); write_char('\n');
 				return;
 			}
@@ -762,6 +769,7 @@ void parse_ingress(void)
 			idx = cmd_words_b[w];
 			uint8_t ret = cmd_parse_port(idx);
 			if (ret != 1) {
+				err_status = ERR_INVALID_ARGUMENT;
 				print_string("Invalid physical port number\n");
 				continue;
 			}
@@ -769,10 +777,12 @@ void parse_ingress(void)
 			idx += ret;
 
 			if (!vlan_ingress_mode_parse(cmd_buffer[idx++], &mode) || !cmd_is_space_or_nul(idx)) {
+				err_status = ERR_INVALID_ARGUMENT;
 				print_string("Invalid ingress mode for port "); print_phys_port(log_port); print_string(" in ingress command\n");
 				goto err;
 			}
 			if (!port_ingress_filter(log_port, mode)) {
+				err_status = ERR_INVALID_ARGUMENT;
 				print_string("Error setting ingress filter for port "); print_phys_port(log_port); write_char('\n');
 				return;
 			}
@@ -783,6 +793,7 @@ void parse_ingress(void)
 	}
 	return;
 err:
+	err_status = ERR_INVALID_ARGUMENT;
 	print_string("Error: ingress [p]<u/t/a>...\n");
 }
 
@@ -848,6 +859,7 @@ void parse_mirror(void)
 	return;
 
 err:
+	err_status = ERR_INVALID_ARGUMENT;
 	print_string("Port/command missing: mirror [status/off/<mirroring port> [port][t/r]]...\n");
 	return;
 }
@@ -856,6 +868,7 @@ err:
 void parse_port(void)
 {
 	if (cmd_words_len < 3) {
+		err_status = ERR_INVALID_ARGUMENT;
 		print_string("\nUsage:" \
 					 "\nport <port> [show|on|off]" \
 					 "\nport <port> [10m|100m|1g|2g5|duplex] [half|full]" \
@@ -864,6 +877,7 @@ void parse_port(void)
 	}
 
 	if (cmd_parse_port_separator(cmd_words_b[1]) == 0) {
+		err_status = ERR_INVALID_ARGUMENT;
 		print_string("Invalid port number\n");
 		return;
 	}
@@ -942,6 +956,7 @@ void parse_port(void)
 			phy_settings.duplex = PHY_DUPLEX_HALF;
 		phy_set_duplex();
 	} else {
+		err_status = ERR_INVALID_ARGUMENT;
 		print_string("Unknown port command\n");
 	}
 }
@@ -970,6 +985,7 @@ void parse_mtu(void)
 	print_byte(p);
 
 	if (atoi_short(cmd_words_b[2]) == 0 || atoi_results_short < 64 || atoi_results_short > 0x3fff) {
+		err_status = ERR_INVALID_ARGUMENT;
 		print_string("MTU must be 64..16383\n");
 		return;
 	}
@@ -979,6 +995,7 @@ void parse_mtu(void)
 	return;
 
 err:
+	err_status = ERR_INVALID_ARGUMENT;
 	print_string("mtu [port] [size]\n");
 	return;
 }
@@ -1036,10 +1053,12 @@ void parse_sfp(void)
 	idx += ret;
 	slot = atoi_results_u8 - 1;
 	if (ret == 0 || !cmd_is_space(idx) || slot > 1) {
+		err_status = ERR_INVALID_ARGUMENT;
 		print_string("Illegal SFP slot number\n");
 		return;
 	}
 	if (slot >= machine.n_sfp) {
+		err_status = ERR_INVALID_ARGUMENT;
 		print_string("SFP slot not present\n");
 		return;
 	}
@@ -1066,6 +1085,7 @@ void parse_sfp(void)
 	handle_sfp();
 	return;
 err:
+	err_status = ERR_INVALID_ARGUMENT;
 	print_string("\nUsage:\n\tsfp\n\tsfp [1|2] [1g|2g5|10g]\n");
 }
 
@@ -1098,6 +1118,7 @@ void parse_regget(void)
 	return;
 
 err:
+	err_status = ERR_INVALID_ARGUMENT;
 	print_string("usage: regget <hexvalue>\n\tlike: regget 0BB0 or regget 0c");
 	return;
 }
@@ -1145,6 +1166,7 @@ void parse_regset(void)
 	return;
 
 err:
+	err_status = ERR_INVALID_ARGUMENT;
 	print_string("usage: regset <hexvalue> <hexvalue>\n\tlike regset 0b abcd1234.");
 }
 
@@ -1188,6 +1210,7 @@ void parse_sdsget(void)
 	return;
 
 err:
+	err_status = ERR_INVALID_ARGUMENT;
 	print_string("usage: sdsget <sds-id> <hex:page> <hex:reg>\n");
 	return;
 }
@@ -1244,6 +1267,7 @@ void parse_sdsset(void)
 	return;
 
 err:
+	err_status = ERR_INVALID_ARGUMENT;
 	print_string("usage: sdsset <sds-id> <hex:page> <hex:reg> <hex:val>\n");
 	return;
 }
@@ -1293,6 +1317,7 @@ void parse_phyget(void)
 	return;
 
 err:
+	err_status = ERR_INVALID_ARGUMENT;
 	print_string("usage: phyget <phy-id> <dev-id> <hex:reg>\n");
 	return;
 }
@@ -1353,6 +1378,7 @@ void parse_physet(void)
 	return;
 
 err:
+	err_status = ERR_INVALID_ARGUMENT;
 	print_string("usage: physet <phy-id> <dev-id> <hex:reg> <hex:val>\n");
 	return;
 }
@@ -1388,6 +1414,7 @@ void parse_passwd(void)
 		passwd[j] = NUL;
 		return;
 	}
+	err_status = ERR_INVALID_ARGUMENT;
 	print_string("Missing password\n");
 }
 
@@ -1412,6 +1439,7 @@ void parse_eee(void)
 		} else if (cmd_is_space_or_nul(idx)) {
 			// Word 2 is a port number
 			if (cmd_parse_port_separator(idx) == 0) {
+				err_status = ERR_INVALID_ARGUMENT;
 				print_string("Speed word invalid, use: [100m|1g|2g5]\n");
 				return;
 			}
@@ -1431,6 +1459,7 @@ void parse_eee(void)
 			speed = EEE_2G5;
 		else 
 		{
+			err_status = ERR_INVALID_ARGUMENT;
 			print_string("Speed word invalid, use: [100m|1g|2g5]\n");
 			return;
 		}
@@ -1451,6 +1480,7 @@ void parse_eee(void)
 		else
 			port_eee_status_all();
 	} else {
+		err_status = ERR_INVALID_ARGUMENT;
 		print_string("eee [on|off|status] [port] [100m|1g|2g5]\n");
 	}
 }
@@ -1522,6 +1552,7 @@ void parse_bw(void)
 	return;
 
 err:
+	err_status = ERR_INVALID_ARGUMENT;
 	print_string("usage: bw [in|out|status] <port> [<hexvalue>|off|drop|fc]\n");
 }
 
@@ -1560,6 +1591,7 @@ void parse_syslog(void)
 			if (was_enabled)
 				syslog_start();
 		} else {
+			err_status = ERR_INVALID_ARGUMENT;
 			print_string("Invalid IP address\n");
 		}
 	} else if (cmd_compare(1, "port")) {
@@ -1571,6 +1603,7 @@ void parse_syslog(void)
 		/* atoi_short() returns zero for no digits and for a value past
 		 * 65535; port zero is not a port either, so both tests are needed. */
 		if (!atoi_short(cmd_words_b[2]) || !atoi_results_short) {
+			err_status = ERR_INVALID_ARGUMENT;
 			print_string("Invalid port\n");
 			return;
 		}
@@ -1587,6 +1620,7 @@ void parse_syslog(void)
 	}
 	else
 	{
+		err_status = ERR_INVALID_ARGUMENT;
 		print_string("Error: syslog [on|off|ip [ip-address]|port [number]]\n");
 		print_string("  on/off enables or disables syslog, ip sets the syslog server IP address,\n");
 		print_string("  port sets the destination UDP port (default 514)\n");
@@ -1757,6 +1791,7 @@ void cmd_parser(void) __banked
 					print_string("Setting ip: ");
 					print_ip(ip); write_char('\n');
 				} else {
+					err_status = ERR_INVALID_ARGUMENT;
 					print_string("Invalid IP address\n" \
 								 "Error: ip [<ip-address>|dhcp]\n" \
 								 "  The dhcp option enables the dhcp client, calling ip without options prints the current IP\n" \
@@ -1773,6 +1808,7 @@ void cmd_parser(void) __banked
 					print_string("Setting gw: ");
 					print_ip(ip); write_char('\n');
 				} else {
+					err_status = ERR_INVALID_ARGUMENT;
 					print_string("Invalid IP address\n" \
 								 "Error: gw <ip-address>\n");
 				}
@@ -1787,6 +1823,7 @@ void cmd_parser(void) __banked
 					print_string("Setting netmask: ");
 					print_ip(ip); write_char('\n');
 				} else {
+					err_status = ERR_INVALID_ARGUMENT;
 					print_string("Invalid IP address\n");
 				}
 			}
@@ -1803,8 +1840,10 @@ void cmd_parser(void) __banked
 				igmp_setup();
 			else if (cmd_compare(1, "show"))
 				igmp_show();
-			else
+			else {
+				err_status = ERR_INVALID_ARGUMENT;
 				print_string("Error: igmp on|off|show\n");
+			}
 		} else if (cmd_compare(0, "mac")) {
 			parse_mac_cmd();
 		} else if (cmd_compare(0, "hostname")) {
@@ -1828,6 +1867,7 @@ void cmd_parser(void) __banked
 				}
 				*dst = NUL;
 			} else {
+				err_status = ERR_INVALID_ARGUMENT;
 				print_string("Error: hostname [name] - the name must not contain spaces\n");
 			}
 		} else if (cmd_compare(0, "stp")) {
@@ -1836,8 +1876,10 @@ void cmd_parser(void) __banked
 			if (cmd_words_len == 3 && cmd_parse_port_separator(cmd_words_b[1]) != 0
 			    && atoi_short(cmd_words_b[2]) && atoi_results_short && atoi_results_short <= 4094)
 				port_pvid_set(atoi_results_u8, atoi_results_short);
-			else
+			else {
+				err_status = ERR_INVALID_ARGUMENT;
 				print_string("Error: pvid <port> <1-4094>\n");
+			}
 		} else if (cmd_compare(0, "vlan")) {
 			parse_vlan();
 		} else if (cmd_compare(0, "isolate")) {
@@ -1897,11 +1939,12 @@ void cmd_parser(void) __banked
 			parse_ingress();
 		}
 		else {
+			err_status = ERR_INVALID_ARGUMENT;
 			print_string("Unknown command\n");
 		}
 
 
-		if (save_cmd && cmd_words_len) {
+		if (save_cmd && cmd_words_len && err_status == ERR_OK) {
 			// Find end of the cmd-buffer, looking for the NUL-byte.
 			uint8_t i = cmd_words_b[cmd_words_len - 1];
 			do {
@@ -1995,6 +2038,7 @@ config_done:
 // If a command is too long or can't be tokenized, remaining commands are not executed
 // Returns the status via `err_status`-variable.
 void execute_commands(__xdata uint8_t *p) __banked {
+	__xdata uint8_t failed = ERR_OK;
 	err_status = ERR_OK;
 	uint8_t cmd_idx = 0;
 	while (1) {
@@ -2005,9 +2049,13 @@ void execute_commands(__xdata uint8_t *p) __banked {
 				if (err_status != ERR_OK)
 					return;
 				cmd_parser();
+				if (failed == ERR_OK)
+					failed = err_status;
 			}
-			if (*p == 0)
+			if (*p == 0) {
+				err_status = failed;
 				return;
+			}
 			cmd_idx = 0;
 		} else {
 			if (cmd_idx < (CMD_BUF_SIZE - 1)) {

rtl837x_common.h

@@ -54,6 +54,7 @@ extern __xdata uint8_t sbuf[SBUF_SIZE];
 #define ERR_OK			0
 #define ERR_TOO_MANY_ARGUMENTS	1
 #define ERR_CMD_TOO_LONG	2
+#define ERR_INVALID_ARGUMENT	3
 
 // For RX data, a propriatary RTL FRAME is inserted. Instead of 0x0800 for IPv4,
 // the RTL_FRAME_TAG_ID is used as part of an 8-byte tag. When VLAN is activated,

rtl837x_stp.c

@@ -15,6 +15,8 @@
 #include "uip.h"
 #include "machine.h"
 
+extern __xdata uint8_t err_status;
+
 // All entry points are __banked and nothing here runs from an interrupt,
 // so the module does not need to stay in the resident bank
 #pragma codeseg BANK2
@@ -1134,5 +1136,6 @@ void stp_parse(void) __banked __reentrant
 	}
 	return;
 err:
+	err_status = ERR_INVALID_ARGUMENT;
 	print_string("Error: stp on|off|status | prio <0-15> | hello <1-10> | maxage <6-40> | fwd <4-30> | txhold <1-10> | version rstp|stp | port <1-9>|lag <1-4> on|off|edge|cost|prio|guard|filter ...\n");
 }