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] 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 []\n"); + cmd_error("Error: mac []\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 | ...))\n"); + cmd_error("Error: lag (show | <1-4> (d | ...))\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 (|show) [port][t]...\n"); + cmd_error("Error: vlan (|show) [port][t]...\n"); } @@ -719,8 +721,7 @@ void parse_isolate(void) return; err: - err_status = ERR_INVALID_ARGUMENT; - print_string("Error: isolate [show|off] [port]...\n"); + cmd_error("Error: isolate [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]...\n"); + cmd_error("Error: ingress [p]...\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/ [port][t/r]]...\n"); + cmd_error("Port/command missing: mirror [status/off/ [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 [show|on|off]" \ "\nport [10m|100m|1g|2g5|duplex] [half|full]" \ "\nport 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 \n\tlike: regget 0BB0 or regget 0c"); + cmd_error("usage: regget \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 \n\tlike regset 0b abcd1234."); + cmd_error("usage: regset \n\tlike regset 0b abcd1234."); } @@ -1210,8 +1195,7 @@ void parse_sdsget(void) return; err: - err_status = ERR_INVALID_ARGUMENT; - print_string("usage: sdsget \n"); + cmd_error("usage: sdsget \n"); return; } @@ -1267,8 +1251,7 @@ void parse_sdsset(void) return; err: - err_status = ERR_INVALID_ARGUMENT; - print_string("usage: sdsset \n"); + cmd_error("usage: sdsset \n"); return; } @@ -1317,8 +1300,7 @@ void parse_phyget(void) return; err: - err_status = ERR_INVALID_ARGUMENT; - print_string("usage: phyget \n"); + cmd_error("usage: phyget \n"); return; } @@ -1378,8 +1360,7 @@ void parse_physet(void) return; err: - err_status = ERR_INVALID_ARGUMENT; - print_string("usage: physet \n"); + cmd_error("usage: physet \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] [|off|drop|fc]\n"); + cmd_error("usage: bw [in|out|status] [|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 [|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 \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 <1-4094>\n"); + cmd_error("Error: pvid <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"); }