From 13cc9db1cea58fad7db154f2aba03fa18932f5f0 Mon Sep 17 00:00:00 2001
From: d00f <8052722+DrDoof@users.noreply.github.com>
Date: Tue, 15 Sep 2026 20:24:32 +0200
Subject: [PATCH 1/4] 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     | 56 ++++++++++++++++++++++++++++++++++++++++++++----
 rtl837x_common.h |  1 +
 rtl837x_stp.c    |  3 +++
 3 files changed, 56 insertions(+), 4 deletions(-)

diff --git a/cmd_parser.c b/cmd_parser.c
index 5a3a3d60..14b4a613 100644
--- a/cmd_parser.c
+++ b/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)) {
diff --git a/rtl837x_common.h b/rtl837x_common.h
index 0ea0e74b..71b5547e 100644
--- a/rtl837x_common.h
+++ b/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,
diff --git a/rtl837x_stp.c b/rtl837x_stp.c
index 14f92387..f7eb871b 100644
--- a/rtl837x_stp.c
+++ b/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");
 }

From c1df7c23d1ac08b149d55b8dc7a0d4e0fe49961b Mon Sep 17 00:00:00 2001
From: d00f <8052722+DrDoof@users.noreply.github.com>
Date: Wed, 16 Sep 2026 16:16:10 +0200
Subject: [PATCH 2/4] cmd: set the error status and print the message in one
 helper

Every rejecting path repeated the same two lines. cmd_error() does both,
which also makes it harder to add a message and forget the status.
---
 cmd_parser.c | 130 +++++++++++++++++++--------------------------------
 1 file changed, 48 insertions(+), 82 deletions(-)

diff --git a/cmd_parser.c b/cmd_parser.c
index 14b4a613..02777bc6 100644
--- a/cmd_parser.c
+++ b/cmd_parser.c
@@ -432,6 +432,13 @@ err:
 // re-registers the static L2 management entry so the change applies without a
 // reboot. As a regular command a "mac ..." line in the startup config applies
 // on every boot (execute_config runs before the final static L2 entry).
+static void cmd_error(__code const char *msg)
+{
+	err_status = ERR_INVALID_ARGUMENT;
+	print_string(msg);
+}
+
+
 void parse_mac_cmd(void)
 {
 	if (cmd_words_len == 1) {
@@ -441,13 +448,11 @@ 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");
+		cmd_error("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");
+		cmd_error("refusing: must be unicast, globally administered\n");
 		return;
 	}
 	if (memcmp(mac_parse_result, uip_ethaddr.addr, 6) == 0) {
@@ -525,8 +530,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");
+	cmd_error("Error: lag (show | <1-4> (d | <port>...))\n");
 }
 
 
@@ -568,8 +572,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");
+	cmd_error("Error: laghash <1-4> [smac|dmac|sip|dip|sport|dport]\n");
 }
 
 
@@ -667,8 +670,7 @@ void parse_vlan(void)
 	}
 	return;
 err:
-	err_status = ERR_INVALID_ARGUMENT;
-	print_string("Error: vlan (<vlan-id>|show) [port][t]...\n");
+	cmd_error("Error: vlan (<vlan-id>|show) [port][t]...\n");
 }
 
 
@@ -719,8 +721,7 @@ void parse_isolate(void)
 	return;
 
 err:
-	err_status = ERR_INVALID_ARGUMENT;
-	print_string("Error: isolate <port> [show|off] [port]...\n");
+	cmd_error("Error: isolate <port> [show|off] [port]...\n");
 }
 
 
@@ -757,8 +758,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');
+				cmd_error("Error setting ingress filter for port "); print_phys_port(log_port); write_char('\n');
 				return;
 			}
 			print_string("All ports ingress filter set to: ");
@@ -769,21 +769,18 @@ 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");
+				cmd_error("Invalid physical port number\n");
 				continue;
 			}
 			log_port = atoi_results_u8;
 			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");
+				cmd_error("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');
+				cmd_error("Error setting ingress filter for port "); print_phys_port(log_port); write_char('\n');
 				return;
 			}
 			print_string("Port "); print_phys_port(log_port);
@@ -793,8 +790,7 @@ void parse_ingress(void)
 	}
 	return;
 err:
-	err_status = ERR_INVALID_ARGUMENT;
-	print_string("Error: ingress [p]<u/t/a>...\n");
+	cmd_error("Error: ingress [p]<u/t/a>...\n");
 }
 
 void parse_mirror(void)
@@ -859,8 +855,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");
+	cmd_error("Port/command missing: mirror [status/off/<mirroring port> [port][t/r]]...\n");
 	return;
 }
 
@@ -868,8 +863,7 @@ err:
 void parse_port(void)
 {
 	if (cmd_words_len < 3) {
-		err_status = ERR_INVALID_ARGUMENT;
-		print_string("\nUsage:" \
+		cmd_error("\nUsage:" \
 					 "\nport <port> [show|on|off]" \
 					 "\nport <port> [10m|100m|1g|2g5|duplex] [half|full]" \
 					 "\nport <port> name [custom port name]\n");
@@ -877,8 +871,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");
+		cmd_error("Invalid port number\n");
 		return;
 	}
 	phy_settings.port = atoi_results_u8;
@@ -956,8 +949,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");
+		cmd_error("Unknown port command\n");
 	}
 }
 
@@ -985,8 +977,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");
+		cmd_error("MTU must be 64..16383\n");
 		return;
 	}
 	REG_WRITE(RTL8373_REG_MAC_L2_PORT_MAX_LEN + ((uint16_t) p << 8), (atoi_results_short >> 10) & 0xf, (atoi_results_short >> 2) & 0xff,
@@ -995,8 +986,7 @@ void parse_mtu(void)
 	return;
 
 err:
-	err_status = ERR_INVALID_ARGUMENT;
-	print_string("mtu [port] [size]\n");
+	cmd_error("mtu [port] [size]\n");
 	return;
 }
 
@@ -1053,13 +1043,11 @@ 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");
+		cmd_error("Illegal SFP slot number\n");
 		return;
 	}
 	if (slot >= machine.n_sfp) {
-		err_status = ERR_INVALID_ARGUMENT;
-		print_string("SFP slot not present\n");
+		cmd_error("SFP slot not present\n");
 		return;
 	}
 
@@ -1085,8 +1073,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");
+	cmd_error("\nUsage:\n\tsfp\n\tsfp [1|2] [1g|2g5|10g]\n");
 }
 
 
@@ -1118,8 +1105,7 @@ void parse_regget(void)
 	return;
 
 err:
-	err_status = ERR_INVALID_ARGUMENT;
-	print_string("usage: regget <hexvalue>\n\tlike: regget 0BB0 or regget 0c");
+	cmd_error("usage: regget <hexvalue>\n\tlike: regget 0BB0 or regget 0c");
 	return;
 }
 
@@ -1166,8 +1152,7 @@ void parse_regset(void)
 	return;
 
 err:
-	err_status = ERR_INVALID_ARGUMENT;
-	print_string("usage: regset <hexvalue> <hexvalue>\n\tlike regset 0b abcd1234.");
+	cmd_error("usage: regset <hexvalue> <hexvalue>\n\tlike regset 0b abcd1234.");
 }
 
 
@@ -1210,8 +1195,7 @@ void parse_sdsget(void)
 	return;
 
 err:
-	err_status = ERR_INVALID_ARGUMENT;
-	print_string("usage: sdsget <sds-id> <hex:page> <hex:reg>\n");
+	cmd_error("usage: sdsget <sds-id> <hex:page> <hex:reg>\n");
 	return;
 }
 
@@ -1267,8 +1251,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");
+	cmd_error("usage: sdsset <sds-id> <hex:page> <hex:reg> <hex:val>\n");
 	return;
 }
 
@@ -1317,8 +1300,7 @@ void parse_phyget(void)
 	return;
 
 err:
-	err_status = ERR_INVALID_ARGUMENT;
-	print_string("usage: phyget <phy-id> <dev-id> <hex:reg>\n");
+	cmd_error("usage: phyget <phy-id> <dev-id> <hex:reg>\n");
 	return;
 }
 
@@ -1378,8 +1360,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");
+	cmd_error("usage: physet <phy-id> <dev-id> <hex:reg> <hex:val>\n");
 	return;
 }
 
@@ -1414,8 +1395,7 @@ void parse_passwd(void)
 		passwd[j] = NUL;
 		return;
 	}
-	err_status = ERR_INVALID_ARGUMENT;
-	print_string("Missing password\n");
+	cmd_error("Missing password\n");
 }
 
 
@@ -1439,8 +1419,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");
+				cmd_error("Speed word invalid, use: [100m|1g|2g5]\n");
 				return;
 			}
 			port = atoi_results_u8;
@@ -1459,8 +1438,7 @@ void parse_eee(void)
 			speed = EEE_2G5;
 		else 
 		{
-			err_status = ERR_INVALID_ARGUMENT;
-			print_string("Speed word invalid, use: [100m|1g|2g5]\n");
+			cmd_error("Speed word invalid, use: [100m|1g|2g5]\n");
 			return;
 		}
 	}
@@ -1480,8 +1458,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");
+		cmd_error("eee [on|off|status] [port] [100m|1g|2g5]\n");
 	}
 }
 
@@ -1552,8 +1529,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");
+	cmd_error("usage: bw [in|out|status] <port> [<hexvalue>|off|drop|fc]\n");
 }
 
 void parse_syslog(void)
@@ -1591,8 +1567,7 @@ void parse_syslog(void)
 			if (was_enabled)
 				syslog_start();
 		} else {
-			err_status = ERR_INVALID_ARGUMENT;
-			print_string("Invalid IP address\n");
+			cmd_error("Invalid IP address\n");
 		}
 	} else if (cmd_compare(1, "port")) {
 		if (cmd_words_len < 3) { // no additional argument -> print current port
@@ -1603,8 +1578,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");
+			cmd_error("Invalid port\n");
 			return;
 		}
 		/* The port is frozen into the connection by uip_udp_new(), so
@@ -1620,8 +1594,7 @@ void parse_syslog(void)
 	}
 	else
 	{
-		err_status = ERR_INVALID_ARGUMENT;
-		print_string("Error: syslog [on|off|ip [ip-address]|port [number]]\n");
+		cmd_error("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");
 	}
@@ -1791,8 +1764,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" \
+					cmd_error("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" \
 								 "  Calling with a valid IP address will stop any ongoing dhcp client and set the IP address\n");
@@ -1808,8 +1780,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" \
+					cmd_error("Invalid IP address\n" \
 								 "Error: gw <ip-address>\n");
 				}
 			}
@@ -1823,8 +1794,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");
+					cmd_error("Invalid IP address\n");
 				}
 			}
 			write_char('\n');
@@ -1841,8 +1811,7 @@ void cmd_parser(void) __banked
 			else if (cmd_compare(1, "show"))
 				igmp_show();
 			else {
-				err_status = ERR_INVALID_ARGUMENT;
-				print_string("Error: igmp on|off|show\n");
+				cmd_error("Error: igmp on|off|show\n");
 			}
 		} else if (cmd_compare(0, "mac")) {
 			parse_mac_cmd();
@@ -1867,8 +1836,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");
+				cmd_error("Error: hostname [name] - the name must not contain spaces\n");
 			}
 		} else if (cmd_compare(0, "stp")) {
 			stp_parse();
@@ -1877,8 +1845,7 @@ void cmd_parser(void) __banked
 			    && atoi_short(cmd_words_b[2]) && atoi_results_short && atoi_results_short <= 4094)
 				port_pvid_set(atoi_results_u8, atoi_results_short);
 			else {
-				err_status = ERR_INVALID_ARGUMENT;
-				print_string("Error: pvid <port> <1-4094>\n");
+				cmd_error("Error: pvid <port> <1-4094>\n");
 			}
 		} else if (cmd_compare(0, "vlan")) {
 			parse_vlan();
@@ -1939,8 +1906,7 @@ void cmd_parser(void) __banked
 			parse_ingress();
 		}
 		else {
-			err_status = ERR_INVALID_ARGUMENT;
-			print_string("Unknown command\n");
+			cmd_error("Unknown command\n");
 		}
 
 

From c39cc421a7f4403af2dbaae8d6161e474ada53d6 Mon Sep 17 00:00:00 2001
From: d00f <8052722+DrDoof@users.noreply.github.com>
Date: Wed, 16 Sep 2026 16:16:10 +0200
Subject: [PATCH 3/4] print: take the message as a pointer to const

The strings are literals in code space, so the parameter can say so.
Drops six warning 357 from the build.
---
 rtl837x_common.h | 6 +++---
 rtlplayground.c  | 6 +++---
 test/support.c   | 2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/rtl837x_common.h b/rtl837x_common.h
index 71b5547e..2ae75fdf 100644
--- a/rtl837x_common.h
+++ b/rtl837x_common.h
@@ -135,9 +135,9 @@ extern __xdata uint8_t uip_buf[UIP_CONF_BUFFER_SIZE+2];
 extern __xdata struct uip_eth_addr uip_ethaddr;
 
 // Headers for calls in the common code area (HOME/BANK0)
-void print_string_no_syslog(__code char *p);
-void print_string_newline_no_syslog(__code char *p);
-void print_string(__code char *p);
+void print_string_no_syslog(__code const char *p);
+void print_string_newline_no_syslog(__code const char *p);
+void print_string(__code const char *p);
 void print_string_x(__xdata char *p);
 void print_long(uint32_t a);
 void print_short(uint16_t a);
diff --git a/rtlplayground.c b/rtlplayground.c
index a2efbf5a..1151d397 100644
--- a/rtlplayground.c
+++ b/rtlplayground.c
@@ -303,19 +303,19 @@ void itoa_short(uint16_t v)
 }
 
 
-void print_string(__code char *p)
+void print_string(__code const char *p)
 {
 	while (*p)
 		write_char(*p++);
 }
 
-void print_string_no_syslog(__code char *p)
+void print_string_no_syslog(__code const char *p)
 {
 	while (*p)
 		write_char_no_syslog(*p++);
 }
 
-void print_string_newline_no_syslog(__code char *p)
+void print_string_newline_no_syslog(__code const char *p)
 {
 	write_char_no_syslog('\n');
 	print_string_no_syslog(p);
diff --git a/test/support.c b/test/support.c
index 58e64ad5..1270554f 100644
--- a/test/support.c
+++ b/test/support.c
@@ -47,7 +47,7 @@ static void out_putc(char c) {
 
 /* --- mocked print helpers (signatures match rtl837x_common.h post-shim) ---- */
 void write_char(char c)            { out_putc(c); }
-void print_string(char *p)         { while (*p) out_putc(*p++); }
+void print_string(const char *p)   { while (*p) out_putc(*p++); }
 void print_string_x(char *p)       { while (*p) out_putc(*p++); }
 void print_cmd_prompt(void)        { print_string((char *)"> "); }
 void itoa(uint8_t v) {                       /* firmware prints a decimal number */

From bece4a20a1addb321ec776eff51e6b3c0995db7f Mon Sep 17 00:00:00 2001
From: d00f <8052722+DrDoof@users.noreply.github.com>
Date: Wed, 16 Sep 2026 23:55:11 +0200
Subject: [PATCH 4/4] cmd: let the helper print the Error prefix

The messages said Error: themselves, or usage:, or nothing at all.
cmd_error() prints the prefix now, so every rejected command answers the
same way and the strings only carry what is specific to them.

Two messages that started with Error or refusing are reworded so the
prefix does not read twice. The syslog usage text becomes one string
instead of three calls.
---
 cmd_parser.c | 59 ++++++++++++++++++++++++++--------------------------
 1 file changed, 30 insertions(+), 29 deletions(-)

diff --git a/cmd_parser.c b/cmd_parser.c
index 02777bc6..d4c4df8b 100644
--- a/cmd_parser.c
+++ b/cmd_parser.c
@@ -426,19 +426,20 @@ err:
 	return 0;
 }
 
+static void cmd_error(__code const char *msg)
+{
+	err_status = ERR_INVALID_ARGUMENT;
+	print_string("Error: ");
+	print_string(msg);
+}
+
+
 // Sets the management MAC from a "mac <aa:bb:cc:dd:ee:ff>" command (or prints
 // the current one with a bare "mac"). Refuses blank/multicast/locally
 // administered/all-zero-OUI addresses so the running MAC stays reachable, and
 // re-registers the static L2 management entry so the change applies without a
 // reboot. As a regular command a "mac ..." line in the startup config applies
 // on every boot (execute_config runs before the final static L2 entry).
-static void cmd_error(__code const char *msg)
-{
-	err_status = ERR_INVALID_ARGUMENT;
-	print_string(msg);
-}
-
-
 void parse_mac_cmd(void)
 {
 	if (cmd_words_len == 1) {
@@ -448,11 +449,11 @@ void parse_mac_cmd(void)
 		return;
 	}
 	if (cmd_words_len != 2 || !parse_mac(cmd_words_b[1])) {
-		cmd_error("Error: mac [<aa:bb:cc:dd:ee:ff>]\n");
+		cmd_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)) {
-		cmd_error("refusing: must be unicast, globally administered\n");
+		cmd_error("the MAC must be unicast and globally administered\n");
 		return;
 	}
 	if (memcmp(mac_parse_result, uip_ethaddr.addr, 6) == 0) {
@@ -530,7 +531,7 @@ void parse_lag(void)
 	port_lag_members_set(group, members);
 	return;
 err:
-	cmd_error("Error: lag (show | <1-4> (d | <port>...))\n");
+	cmd_error("lag (show | <1-4> (d | <port>...))\n");
 }
 
 
@@ -572,7 +573,7 @@ void parse_lag_hash(void)
 	port_lag_hash_set(group, hash);
 	return;
 err:
-	cmd_error("Error: laghash <1-4> [smac|dmac|sip|dip|sport|dport]\n");
+	cmd_error("laghash <1-4> [smac|dmac|sip|dip|sport|dport]\n");
 }
 
 
@@ -670,7 +671,7 @@ void parse_vlan(void)
 	}
 	return;
 err:
-	cmd_error("Error: vlan (<vlan-id>|show) [port][t]...\n");
+	cmd_error("vlan (<vlan-id>|show) [port][t]...\n");
 }
 
 
@@ -721,7 +722,7 @@ void parse_isolate(void)
 	return;
 
 err:
-	cmd_error("Error: isolate <port> [show|off] [port]...\n");
+	cmd_error("isolate <port> [show|off] [port]...\n");
 }
 
 
@@ -758,7 +759,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)) {
-				cmd_error("Error setting ingress filter for port "); print_phys_port(log_port); write_char('\n');
+				cmd_error("cannot set the ingress filter for port "); print_phys_port(log_port); write_char('\n');
 				return;
 			}
 			print_string("All ports ingress filter set to: ");
@@ -780,7 +781,7 @@ void parse_ingress(void)
 				goto err;
 			}
 			if (!port_ingress_filter(log_port, mode)) {
-				cmd_error("Error setting ingress filter for port "); print_phys_port(log_port); write_char('\n');
+				cmd_error("cannot set the ingress filter for port "); print_phys_port(log_port); write_char('\n');
 				return;
 			}
 			print_string("Port "); print_phys_port(log_port);
@@ -790,7 +791,7 @@ void parse_ingress(void)
 	}
 	return;
 err:
-	cmd_error("Error: ingress [p]<u/t/a>...\n");
+	cmd_error("ingress [p]<u/t/a>...\n");
 }
 
 void parse_mirror(void)
@@ -1105,7 +1106,7 @@ void parse_regget(void)
 	return;
 
 err:
-	cmd_error("usage: regget <hexvalue>\n\tlike: regget 0BB0 or regget 0c");
+	cmd_error("regget <hexvalue>\n\tlike: regget 0BB0 or regget 0c");
 	return;
 }
 
@@ -1152,7 +1153,7 @@ void parse_regset(void)
 	return;
 
 err:
-	cmd_error("usage: regset <hexvalue> <hexvalue>\n\tlike regset 0b abcd1234.");
+	cmd_error("regset <hexvalue> <hexvalue>\n\tlike regset 0b abcd1234.");
 }
 
 
@@ -1195,7 +1196,7 @@ void parse_sdsget(void)
 	return;
 
 err:
-	cmd_error("usage: sdsget <sds-id> <hex:page> <hex:reg>\n");
+	cmd_error("sdsget <sds-id> <hex:page> <hex:reg>\n");
 	return;
 }
 
@@ -1251,7 +1252,7 @@ void parse_sdsset(void)
 	return;
 
 err:
-	cmd_error("usage: sdsset <sds-id> <hex:page> <hex:reg> <hex:val>\n");
+	cmd_error("sdsset <sds-id> <hex:page> <hex:reg> <hex:val>\n");
 	return;
 }
 
@@ -1300,7 +1301,7 @@ void parse_phyget(void)
 	return;
 
 err:
-	cmd_error("usage: phyget <phy-id> <dev-id> <hex:reg>\n");
+	cmd_error("phyget <phy-id> <dev-id> <hex:reg>\n");
 	return;
 }
 
@@ -1360,7 +1361,7 @@ void parse_physet(void)
 	return;
 
 err:
-	cmd_error("usage: physet <phy-id> <dev-id> <hex:reg> <hex:val>\n");
+	cmd_error("physet <phy-id> <dev-id> <hex:reg> <hex:val>\n");
 	return;
 }
 
@@ -1529,7 +1530,7 @@ void parse_bw(void)
 	return;
 
 err:
-	cmd_error("usage: bw [in|out|status] <port> [<hexvalue>|off|drop|fc]\n");
+	cmd_error("bw [in|out|status] <port> [<hexvalue>|off|drop|fc]\n");
 }
 
 void parse_syslog(void)
@@ -1594,9 +1595,9 @@ void parse_syslog(void)
 	}
 	else
 	{
-		cmd_error("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");
+		cmd_error("syslog [on|off|ip [ip-address]|port [number]]\n"
+			  "  on/off enables or disables syslog, ip sets the syslog server IP address,\n"
+			  "  port sets the destination UDP port (default 514)\n");
 	}
 }
 
@@ -1811,7 +1812,7 @@ void cmd_parser(void) __banked
 			else if (cmd_compare(1, "show"))
 				igmp_show();
 			else {
-				cmd_error("Error: igmp on|off|show\n");
+				cmd_error("igmp on|off|show\n");
 			}
 		} else if (cmd_compare(0, "mac")) {
 			parse_mac_cmd();
@@ -1836,7 +1837,7 @@ void cmd_parser(void) __banked
 				}
 				*dst = NUL;
 			} else {
-				cmd_error("Error: hostname [name] - the name must not contain spaces\n");
+				cmd_error("hostname [name] - the name must not contain spaces\n");
 			}
 		} else if (cmd_compare(0, "stp")) {
 			stp_parse();
@@ -1845,7 +1846,7 @@ void cmd_parser(void) __banked
 			    && atoi_short(cmd_words_b[2]) && atoi_results_short && atoi_results_short <= 4094)
 				port_pvid_set(atoi_results_u8, atoi_results_short);
 			else {
-				cmd_error("Error: pvid <port> <1-4094>\n");
+				cmd_error("pvid <port> <1-4094>\n");
 			}
 		} else if (cmd_compare(0, "vlan")) {
 			parse_vlan();