From 4937618684a29e1395e501f6fcc56907352bd2b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Tue, 11 Aug 2026 20:50:36 +0200 Subject: [PATCH 01/26] Change atoi_short() so it returns number of bytes consumed. --- cmd_parser.c | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index 842d3f5..7a46a52 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -80,6 +80,8 @@ __xdata uint16_t cmd_history_ptr; // Error set by commands __xdata uint8_t err_status; +__xdata uint16_t atoi_results_short; + inline uint8_t isletter(uint8_t l) { // return (l >= 'a' && l <= 'z') || (l >= 'A' && l <= 'Z'); @@ -203,22 +205,26 @@ uint8_t atoi_byte(__xdata uint8_t *out, uint8_t idx) return err; } - -uint8_t atoi_short(__xdata uint16_t *vlan, uint8_t idx) +// return 0 on error or non-zero number of number-byte taken for the conversion. +// Stops at any non-digit '0'-'9' char or bytes is more then 5. +uint8_t atoi_short(uint8_t idx) { - uint8_t err = 1; - *vlan = 0; + uint8_t cnt = 0; + atoi_results_short = 0; - while (isnumber(cmd_buffer[idx])) { - err = 0; - uint8_t val = cmd_buffer[idx] - '0'; - if (*vlan > 6553 || (*vlan == 6553 && val > 5)) - return 1; - *vlan = (*vlan * 10) + val; - idx++; + uint8_t *ptr = &cmd_buffer[idx]; + + while (1) { + uint8_t val = *ptr++ - '0'; + if (val > 9) + break; + if (atoi_results_short > 6553 || (atoi_results_short == 6553 && val > 5) || cnt >= 5) + return 0; + atoi_results_short = (atoi_results_short * 10) + val; + cnt++; } - return err; + return cnt; } From c265a20d8380d6d31e60412cb1fe64965f580c38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Tue, 11 Aug 2026 21:12:07 +0200 Subject: [PATCH 02/26] Refactor the code for the new atoi_short() Free-up 1 SRAM-byte --- cmd_parser.c | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index 7a46a52..8c9db08 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -355,11 +355,16 @@ void parse_vlan(void) vlan_settings.vlan = 0; vlan_settings.members = 0; vlan_settings.tagged = 0; + if (cmd_words_len < 2) goto err; - if (!atoi_short(&vlan_settings.vlan, cmd_words_b[1])) { + + uint8_t ret = atoi_short(cmd_words_b[1]); + if (ret) { if (cmd_words_len == 3 && cmd_buffer[cmd_words_b[2]] == 'd') { - vlan_delete(vlan_settings.vlan); + if (atoi_results_short > 4094) + goto err; + vlan_delete(atoi_results_short); return; } if (cmd_compare(2, "mgmt")) { @@ -376,7 +381,7 @@ void parse_vlan(void) goto err; uint8_t w = 2; if (cmd_words_len > w && isletter(cmd_buffer[cmd_words_b[w]])) { - register uint8_t i = 0; + uint8_t i = 0; vlan_name_remove(vlan_settings.vlan); vlan_names[vlan_ptr++] = hex[(vlan_settings.vlan >> 8) & 0xf]; vlan_names[vlan_ptr++] = hex[(vlan_settings.vlan >> 4) & 0xf] ; @@ -737,7 +742,7 @@ void parse_port(void) void parse_mtu(void) { __xdata uint16_t mtu; - uint8_t p; + uint8_t p, ret; if (cmd_compare(1, "show")) { for (p = machine.min_port; p <= machine.max_port; p++) { @@ -756,12 +761,15 @@ void parse_mtu(void) } p = machine.phys_to_log_port[cmd_buffer[cmd_words_b[1]] - '1']; print_byte(p); - if (atoi_short(&mtu, cmd_words_b[2]) || mtu < 64 || mtu > 0x3fff) { + + ret = atoi_short(cmd_words_b[2]); + + if (!ret || atoi_results_short < 64 || atoi_results_short > 0x3fff) { print_string("MTU must be 64..16383\n"); return; } - REG_WRITE(RTL8373_REG_MAC_L2_PORT_MAX_LEN + ((uint16_t) p << 8), (mtu >> 10) & 0xf, (mtu >> 2) & 0xff, - ((mtu & 0x3) << 6) | ((mtu >> 8) & 0x3f), mtu & 0xff); + REG_WRITE(RTL8373_REG_MAC_L2_PORT_MAX_LEN + ((uint16_t) p << 8), (atoi_results_short >> 10) & 0xf, (atoi_results_short >> 2) & 0xff, + ((atoi_results_short & 0x3) << 6) | ((atoi_results_short >> 8) & 0x3f), atoi_results_short & 0xff); write_char('\n'); } @@ -1597,12 +1605,12 @@ void cmd_parser(void) __banked stpEnabled = 0; } } else if (cmd_compare(0, "pvid") && cmd_words_len == 3) { - __xdata uint16_t pvid; + uint8_t ret; if (cmd_buffer[cmd_words_b[1]] >= '1' && cmd_buffer[cmd_words_b[1]] <= '9' && cmd_buffer[cmd_words_b[1] + 1] <= ' ' - && !atoi_short(&pvid, cmd_words_b[2]) && pvid && pvid <= 4094) - port_pvid_set(machine.phys_to_log_port[cmd_buffer[cmd_words_b[1]] - '1'], pvid); + && atoi_short(cmd_words_b[2]) && atoi_results_short && atoi_results_short <= 4094) + port_pvid_set(machine.phys_to_log_port[cmd_buffer[cmd_words_b[1]] - '1'], atoi_results_short); else print_string("Error: pvid <1-4094>\n"); } else if (cmd_compare(0, "vlan")) { From 80f97a50e0ed151dbd79e0b7bf4e867ba7aecd21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Tue, 11 Aug 2026 21:15:48 +0200 Subject: [PATCH 03/26] Refactor atoi_byte() same as atoi_short() --- cmd_parser.c | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index 8c9db08..c09d752 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -81,6 +81,7 @@ __xdata uint16_t cmd_history_ptr; __xdata uint8_t err_status; __xdata uint16_t atoi_results_short; +__xdata uint8_t atoi_results_u8; inline uint8_t isletter(uint8_t l) { @@ -187,24 +188,30 @@ uint8_t atoi_hex(uint8_t idx) } -uint8_t atoi_byte(__xdata uint8_t *out, uint8_t idx) +// return 0 on error or non-zero number of number-byte taken for the conversion. +// Stops at any non-digit '0'-'9' char or bytes is more then 3. +uint8_t atoi_byte(uint8_t idx) { - uint8_t err = 1; + uint8_t cnt = 0; uint8_t num = 0; - while (isnumber(cmd_buffer[idx])) { - uint8_t val = cmd_buffer[idx] - '0'; - err = 0; - if (num > 25 || (num == 25 && val > 5)) - return 1; + uint8_t * ptr = &cmd_buffer[idx]; + + while (1) { + uint8_t val = *ptr++ - '0'; + if (val > 9) + break; + if (num > 25 || (num == 25 && val > 5) || cnt >= 3) + return 0; num = (num * 10) + val; - idx++; + cnt++; } - *out = num; - return err; + atoi_results_u8 = num; + return cnt; } + // return 0 on error or non-zero number of number-byte taken for the conversion. // Stops at any non-digit '0'-'9' char or bytes is more then 5. uint8_t atoi_short(uint8_t idx) From 57061161b6898fde94e45c6af21fa80c5b758c74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Tue, 11 Aug 2026 21:32:04 +0200 Subject: [PATCH 04/26] Refactor code to make use of new atoi_byte() --- cmd_parser.c | 89 ++++++++++++++++++++++++++++++++++------------------ 1 file changed, 58 insertions(+), 31 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index c09d752..269a6d8 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -235,22 +235,32 @@ uint8_t atoi_short(uint8_t idx) } -uint8_t parse_ip(uint8_t idx) +int8_t parse_ip(uint8_t idx) { - __xdata uint8_t b; + uint8_t b = 0; + uint8_t ret; - for (b = 0; b < 4; b++) { - ip[b] = 0; - while (isnumber(cmd_buffer[idx])) { - ip[b] = (ip[b] * 10) + cmd_buffer[idx] - '0'; - idx++; + while(1) { + ret = atoi_byte(idx); + if (ret == 0) { + goto err; } - if (b < 3 && cmd_buffer[idx++] != '.') { - print_string("Error in IP format, expecting '.'\n"); - return -1; + idx += ret; + ip[b++] = atoi_results_u8; + + if (b == 4) { + break; + } + + if (cmd_buffer[idx++] != '.') { + goto err; } } return 0; + +err: + print_string("Error in IP format\n"); + return -1; } @@ -258,6 +268,7 @@ void parse_lag(void) { __xdata uint8_t group; __xdata uint16_t members = 0; + uint8_t ret; if (cmd_compare(1, "show")) { print_string("LAG status:\n"); @@ -284,30 +295,35 @@ void parse_lag(void) return; } - if (cmd_words_len < 2 || !isnumber(cmd_buffer[cmd_words_b[1]])) + if (cmd_words_len < 2) goto err; - group = cmd_buffer[cmd_words_b[1]] - '1'; + + // Parse group, expect only one number 0-9. + ret = atoi_byte(cmd_words_b[1]); + if (ret != 1) { + goto err; + } + group = atoi_results_u8 - 1; + if (group > 3) /* '0' wraps well past three, so one test does both ends */ goto err; + uint8_t w = 2; while (w < cmd_words_len) { // write_char('|'); print_byte(w); write_char(':'); write_char(cmd_buffer[cmd_words_b[w]]); write_char('-'); - uint8_t port; - if (isnumber(cmd_buffer[cmd_words_b[w]])) { - port = cmd_buffer[cmd_words_b[w]] - '1'; - if (isnumber(cmd_buffer[cmd_words_b[w] + 1])) - port = (port + 1) * 10 + cmd_buffer[cmd_words_b[w] + 1] - '1'; - if (port > 8) /* phys_to_log_port holds nine entries */ - goto err; - port = machine.phys_to_log_port[port]; - } else { + + // Parse port. + ret = atoi_byte(cmd_words_b[w++]); + if (ret != 1) goto err; - } + + uint8_t port = atoi_results_u8; + port = machine.phys_to_log_port[port]; + if (port > machine.max_port) goto err; members |= ((uint16_t)1) << port; - w++; } port_lag_members_set(group, members); return; @@ -828,11 +844,14 @@ void parse_sfp(void) } return; } - if (cmd_buffer[cmd_words_b[1]] < '1' || cmd_buffer[cmd_words_b[1]] > '2' || cmd_buffer[cmd_words_b[1] + 1] != ' ' ) { + uint8_t idx = cmd_words_b[1]; + uint8_t ret = atoi_byte(idx); + idx += ret; + slot = atoi_results_u8 - 1; + if (ret == 0 || cmd_buffer[idx] != ' ' || slot > 1) { print_string("Illegal SFP slot number\n"); return; } - slot = cmd_buffer[cmd_words_b[1]] - '1'; if (slot >= machine.n_sfp) { print_string("SFP slot not present\n"); return; @@ -955,9 +974,10 @@ void parse_sdsget(void) goto err; } - if (atoi_byte(&sds_id, cmd_words_b[1])) { + if (!atoi_byte(cmd_words_b[1])) { goto err; } + sds_id = atoi_results_u8; hex_size = atoi_hex(cmd_words_b[2]); if (hex_size != 1) { @@ -999,9 +1019,10 @@ void parse_sdsset(void) goto err; } - if (atoi_byte(&sds_id, cmd_words_b[1])) { + if (!atoi_byte(cmd_words_b[1])) { goto err; } + sds_id = atoi_results_u8; hex_size = atoi_hex(cmd_words_b[2]); if (hex_size != 1) { @@ -1054,13 +1075,16 @@ void parse_phyget(void) goto err; } - if (atoi_byte(&phy_id, cmd_words_b[1])) { + if (!atoi_byte(cmd_words_b[1])) { goto err; } + phy_id = atoi_results_u8; - if (atoi_byte(&dev_id, cmd_words_b[2])) { + + if (!atoi_byte(cmd_words_b[2])) { goto err; } + dev_id = atoi_results_u8; hex_size = atoi_hex(cmd_words_b[3]); if (hex_size == 0 || hex_size > 2) { @@ -1100,13 +1124,16 @@ void parse_physet(void) goto err; } - if (atoi_byte(&phy_id, cmd_words_b[1])) { + if (!atoi_byte(cmd_words_b[1])) { goto err; } + phy_id = atoi_results_u8; - if (atoi_byte(&dev_id, cmd_words_b[2])) { + + if (!atoi_byte(cmd_words_b[2])) { goto err; } + dev_id = atoi_results_u8; hex_size = atoi_hex(cmd_words_b[3]); if (hex_size == 0 || hex_size > 2) { From 175f3bb1856ecf3456124d5ea92f430262c80e87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Tue, 11 Aug 2026 21:44:10 +0200 Subject: [PATCH 05/26] Add cmd_parse_port() and cmd_parse_port_space() helper. A lot of places we manual parse and translate the port. These helper functions will do that for us. --- cmd_parser.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/cmd_parser.c b/cmd_parser.c index 269a6d8..9d627e7 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -234,6 +234,55 @@ uint8_t atoi_short(uint8_t idx) return cnt; } +/* Parse, validate and translate phys_to_log_port port argument + * cpu_is_valid tells the parser that port 10 is valid input. + * returns 0 when on parser error or invalid value. + * return non-zero number of bytes consumed. + * Store the value in atoi_results_u8. + */ +uint8_t cmd_parse_port(uint8_t idx, __bit cpu_is_valid) { + uint8_t ret = atoi_byte(idx); + uint8_t port = 0; + if (ret != 0) { + port = atoi_results_u8 - 1; + + if (cpu_is_valid && port == 9) { + // CPU port is valid + } else if (port < 9) { + port = machine.phys_to_log_port[port]; + if (port < machine.min_port || port > machine.max_port) { + ret = 0; + } + } else { + ret = 0; + } + } + + atoi_results_u8 = port; + return ret; +} + + +// Same as cmd_parse_port() addition to check the trailing space. +// returns 0 when on parser error or invalid value or no space. +// return non-zero number of bytes consumed including the space. +uint8_t cmd_parse_port_space(uint8_t idx, __bit cpu_is_valid) { + uint8_t ret = cmd_parse_port(idx, cpu_is_valid); + if (ret != 0) { + idx += ret; + ret++; + if (cmd_buffer[idx] != ' ') + ret = 0; + } + return ret; +} + + +// check if the cmd_buffer[idx] is a space. +__bit cmd_is_space(uint8_t idx) { + return cmd_buffer[idx] == ' '; +} + int8_t parse_ip(uint8_t idx) { From 73df006d392ed8e489332652a0da8d48f4b29ad3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Tue, 11 Aug 2026 22:55:07 +0200 Subject: [PATCH 06/26] Replace all manual port parsing with cmd_parse_port_space() or cmd_parse_port(). Saves no SRAM but around 1k code size --- cmd_parser.c | 247 ++++++++++++++++++++++----------------------------- 1 file changed, 107 insertions(+), 140 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index 9d627e7..296741b 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -349,30 +349,22 @@ void parse_lag(void) // Parse group, expect only one number 0-9. ret = atoi_byte(cmd_words_b[1]); - if (ret != 1) { - goto err; - } - group = atoi_results_u8 - 1; - - if (group > 3) /* '0' wraps well past three, so one test does both ends */ + if (ret != 1) goto err; + group = atoi_results_u8 - 1; + if (group > 3) /* '0' wraps well past three, so one test does both ends */ + goto err; uint8_t w = 2; while (w < cmd_words_len) { // write_char('|'); print_byte(w); write_char(':'); write_char(cmd_buffer[cmd_words_b[w]]); write_char('-'); // Parse port. - ret = atoi_byte(cmd_words_b[w++]); - if (ret != 1) + if (cmd_parse_port(cmd_words_b[w++], false) == 0) goto err; - uint8_t port = atoi_results_u8; - port = machine.phys_to_log_port[port]; - - if (port > machine.max_port) - goto err; - members |= ((uint16_t)1) << port; + members |= ((uint16_t)1) << atoi_results_u8; } port_lag_members_set(group, members); return; @@ -431,17 +423,13 @@ void parse_vlan(void) if (cmd_words_len < 2) goto err; - uint8_t ret = atoi_short(cmd_words_b[1]); - if (ret) { - if (cmd_words_len == 3 && cmd_buffer[cmd_words_b[2]] == 'd') { - if (atoi_results_short > 4094) - goto err; - vlan_delete(atoi_results_short); - return; - } + // Parse the VLAN number + if (atoi_short(cmd_words_b[1]) != 0) { + if (atoi_results_short > 4094) + goto err; + vlan_settings.vlan = atoi_results_short; + if (cmd_compare(2, "mgmt")) { - if (vlan_settings.vlan > 4094) - goto err; management_vlan = vlan_settings.vlan; if (!vlan_settings.vlan) print_string("Management VLAN disabled\n"); @@ -449,8 +437,16 @@ void parse_vlan(void) print_string("Management VLAN set to "); print_short(management_vlan); write_char('\n'); return; } - if (!vlan_settings.vlan || vlan_settings.vlan > 4094) + + // Other commands vlan 0 is invalid + if (!vlan_settings.vlan) goto err; + + if (cmd_words_len == 3 && cmd_buffer[cmd_words_b[2]] == 'd') { + vlan_delete(vlan_settings.vlan); + return; + } + uint8_t w = 2; if (cmd_words_len > w && isletter(cmd_buffer[cmd_words_b[w]])) { uint8_t i = 0; @@ -466,24 +462,21 @@ void parse_vlan(void) w++; print_string("<\n"); } + + uint8_t ret; + uint8_t idx; while (cmd_words_len > w) { - __xdata uint8_t port; - if (isnumber(cmd_buffer[cmd_words_b[w]])) { - port = cmd_buffer[cmd_words_b[w]] - '1'; - if (isnumber(cmd_buffer[cmd_words_b[w] + 1])) { - port = (port + 1) * 10 + cmd_buffer[cmd_words_b[w] + 1] - '1'; - if (cmd_buffer[cmd_words_b[w] + 2] == 't') - vlan_settings.tagged |= ((uint16_t)1) << port; - } else { - port = machine.phys_to_log_port[port]; - if (cmd_buffer[cmd_words_b[w] + 1] == 't') - vlan_settings.tagged |= ((uint16_t)1) << port; - } - if (port > machine.max_port) - goto err; - vlan_settings.members |= ((uint16_t)1) << port; - } - w++; + idx = cmd_words_b[w++]; + ret = cmd_parse_port(idx, false); + if (ret == 0) + goto err; + + idx += ret; + uint16_t pmask = ((uint16_t)1) << atoi_results_u8; + vlan_settings.members |= pmask; + + if (cmd_buffer[idx] == 't') + vlan_settings.tagged |= pmask; } vlan_create(); } else if (cmd_compare(1, "show")) { @@ -511,12 +504,9 @@ void parse_isolate(void) print_string("\nISOLATE "); - if (!isnumber(cmd_buffer[cmd_words_b[1]]) || cmd_buffer[cmd_words_b[1]] == '0' - || isnumber(cmd_buffer[cmd_words_b[1] + 1])) - goto err; - __xdata uint8_t port_configured = machine.phys_to_log_port[cmd_buffer[cmd_words_b[1]] - '1']; - if (port_configured < machine.min_port || port_configured > machine.max_port) + if (cmd_parse_port_space(cmd_words_b[1], false) == 0) goto err; + uint8_t port_configured = atoi_results_u8; print_byte(port_configured); write_char('\n'); @@ -545,22 +535,12 @@ void parse_isolate(void) uint8_t w = 2; while (w < cmd_words_len) { - __xdata uint8_t port; - if (isnumber(cmd_buffer[cmd_words_b[w]])) { - port = cmd_buffer[cmd_words_b[w]] - '1'; - if (isnumber(cmd_buffer[cmd_words_b[w] + 1])) { - port = (port + 1) * 10 + cmd_buffer[cmd_words_b[w] + 1] - '1'; // logical port - if (port != 9) // CPU port is logical port 9 - goto err; - } else { - port = machine.phys_to_log_port[port]; - if (port < machine.min_port || port > machine.max_port) - goto err; - } - members |= ((uint16_t)1) << port; - } - w++; + if (cmd_parse_port(cmd_words_b[w++], false) == 0) + goto err; + uint8_t port = atoi_results_u8; + members |= ((uint16_t)1) << port; } + port_isolate(port_configured, members); return; @@ -605,19 +585,19 @@ void parse_ingress(void) print_string("All ports ingress filter set to: "); print_port_ingress_filter_mode(mode); write_char('\n'); } - return; } else { for(uint8_t w = 1; w < cmd_words_len; w++) { - uint8_t p = cmd_buffer[cmd_words_b[w]]; - if (!isnumber(p)) { + uint8_t idx = cmd_words_b[w]; + char p = cmd_buffer[idx]; + uint8_t ret = cmd_parse_port_space(idx, false); + if (ret == 0) { + print_string("Invalid physical port number\n"); continue; } - if (p < '1') { - print_string("Invalid physical port number: "); write_char(p); write_char('\n'); - continue; - } - log_port = machine.phys_to_log_port[p - '1']; - if (!vlan_ingress_mode_parse(cmd_buffer[cmd_words_b[w] + 1], &mode)) { + log_port = atoi_results_u8; + idx += ret; + + if (!vlan_ingress_mode_parse(cmd_buffer[idx], &mode)) { print_string("Invalid ingress mode for port "); write_char(p); print_string(" in ingress command\n"); goto err; } @@ -629,15 +609,15 @@ void parse_ingress(void) print_string(" ingress filter set to: "); print_port_ingress_filter_mode(mode); write_char('\n'); } - return; } + return; err: print_string("Error: ingress [p]... \n"); } void parse_mirror(void) { - __xdata uint8_t mirroring_port; + __xdata uint8_t mirroring_port = 0; __xdata uint16_t rx_pmask = 0; __xdata uint16_t tx_pmask = 0; @@ -667,48 +647,38 @@ void parse_mirror(void) return; } - if (cmd_words_len < 2 || !isnumber(cmd_buffer[cmd_words_b[1]])) { - print_string("Port/command missing: mirror [status/off/ [port][t/r]]...\n"); - return; - } - - mirroring_port = cmd_buffer[cmd_words_b[1]] - '1'; - if (isnumber(cmd_buffer[cmd_words_b[1] + 1])) - mirroring_port = (mirroring_port + 1) * 10 + cmd_buffer[cmd_words_b[1] + 1] - '1'; - mirroring_port = machine.phys_to_log_port[mirroring_port]; - + if (cmd_words_len < 2) + goto err; uint8_t w = 2; + uint8_t port; + uint8_t ret; while (w < cmd_words_len) { - uint8_t port; - if (isnumber(cmd_buffer[cmd_words_b[w]])) { - port = cmd_buffer[cmd_words_b[w]] - '1'; - if (isnumber(cmd_buffer[cmd_words_b[w] + 1])) { - port = (port + 1) * 10 + cmd_buffer[cmd_words_b[w] + 1] - '1'; - port = machine.phys_to_log_port[port]; - if (cmd_buffer[cmd_words_b[w] + 2] == 'r') - rx_pmask |= ((uint16_t)1) << port; - else if (cmd_buffer[cmd_words_b[w] + 2] == 't') - tx_pmask |= ((uint16_t)1) << port; - else { - rx_pmask |= ((uint16_t)1) << port; - tx_pmask |= ((uint16_t)1) << port; - } - } else { - port = machine.phys_to_log_port[port]; - if (cmd_buffer[cmd_words_b[w] + 1] == 'r') - rx_pmask |= ((uint16_t)1) << port; - else if (cmd_buffer[cmd_words_b[w] + 1] == 't') - tx_pmask |= ((uint16_t)1) << port; - else { - rx_pmask |= ((uint16_t)1) << port; - tx_pmask |= ((uint16_t)1) << port; - } - } - } - w++; + uint8_t idx = cmd_words_b[w++]; + ret = cmd_parse_port(idx, false); + if (ret == 0) + goto err; + + idx += ret; + port = atoi_results_u8; + + // Use the first port argument as mirroring_port + if (w == 2) + mirroring_port = port; + + ret = cmd_buffer[idx]; + uint16_t pmask = ((uint16_t)1) << port; + if (ret != 't') + rx_pmask |= pmask; + if (ret != 'r') + tx_pmask |= pmask; } port_mirror_set(mirroring_port, rx_pmask, tx_pmask); + return; + +err: + print_string("Port/command missing: mirror [status/off/ [port][t/r]]...\n"); + return; } @@ -722,16 +692,11 @@ void parse_port(void) return; } - if (cmd_buffer[cmd_words_b[1]] < '1' || cmd_buffer[cmd_words_b[1]] > '9' || cmd_buffer[cmd_words_b[1] + 1] != ' ' ) { - print_string("Illegal port number\n"); - return; - } - phy_settings.port = cmd_buffer[cmd_words_b[1]] - '1'; - phy_settings.port = machine.phys_to_log_port[phy_settings.port]; - if (phy_settings.port > machine.max_port || phy_settings.port < machine.min_port) { - print_string("This machine has no port with the specified number\n"); + if (cmd_parse_port_space(cmd_words_b[1], false) == 0) { + print_string("Invalid port number\n"); return; } + phy_settings.port = atoi_results_u8; print_string("Logical Port: "); print_byte(phy_settings.port); write_char('\n'); phy_settings.duplex = PHY_DUPLEX_BOTH; @@ -813,25 +778,24 @@ void parse_port(void) void parse_mtu(void) { - __xdata uint16_t mtu; uint8_t p, ret; if (cmd_compare(1, "show")) { for (p = machine.min_port; p <= machine.max_port; p++) { reg_read_m(RTL8373_REG_MAC_L2_PORT_MAX_LEN + ((uint16_t) p << 8)); - mtu = SFR_DATA_U16 & 0x3fff; + uint16_t mtu = SFR_DATA_U16 & 0x3fff; print_string("Port "); print_byte(machine.log_to_phys_port[p]); write_char(' '); print_short(mtu); write_char('\n'); } return; } - if (cmd_words_len != 3 || cmd_buffer[cmd_words_b[1]] < '1' - || cmd_buffer[cmd_words_b[1]] > '9' - || cmd_buffer[cmd_words_b[1] + 1] > ' ') { - print_string("mtu [port] [size]\n"); - return; - } - p = machine.phys_to_log_port[cmd_buffer[cmd_words_b[1]] - '1']; + if (cmd_words_len != 3) + goto err; + + if (cmd_parse_port_space(cmd_words_b[1], false) == 0) + goto err; + + p = atoi_results_u8; print_byte(p); ret = atoi_short(cmd_words_b[2]); @@ -843,6 +807,11 @@ void parse_mtu(void) REG_WRITE(RTL8373_REG_MAC_L2_PORT_MAX_LEN + ((uint16_t) p << 8), (atoi_results_short >> 10) & 0xf, (atoi_results_short >> 2) & 0xff, ((atoi_results_short & 0x3) << 6) | ((atoi_results_short >> 8) & 0x3f), atoi_results_short & 0xff); write_char('\n'); + return; + +err: + print_string("mtu [port] [size]\n"); + return; } bool sfp_print_measurements(uint8_t sfp) @@ -897,7 +866,7 @@ void parse_sfp(void) uint8_t ret = atoi_byte(idx); idx += ret; slot = atoi_results_u8 - 1; - if (ret == 0 || cmd_buffer[idx] != ' ' || slot > 1) { + if (ret == 0 || !cmd_is_space(idx) || slot > 1) { print_string("Illegal SFP slot number\n"); return; } @@ -1277,8 +1246,11 @@ void parse_eee(void) speed_word = 2; } else if (cmd_buffer[idx] == ' ' || cmd_buffer[idx] == '\0') { // Word 2 is a port number - port = cmd_buffer[cmd_words_b[2]] - '1'; - port = machine.phys_to_log_port[port]; + if (cmd_parse_port_space(idx, false) == 0) { + print_string("Speed word invalid, use: [100m|1g|2g5]\n"); + return; + } + port = atoi_results_u8; // Check if word 3 is a speed if (cmd_words_len >= 4) speed_word = 3; @@ -1321,17 +1293,15 @@ void parse_eee(void) void parse_bw(void) { - __xdata uint8_t port; + uint8_t port; __xdata uint32_t bw = 0; if (cmd_words_len < 2) // Check for at least 2 arguments goto err; - port = cmd_buffer[cmd_words_b[2]] - '1'; - if (port > 9) + if (cmd_parse_port_space(cmd_words_b[2], false) == 0) goto err; - - port = machine.phys_to_log_port[port]; + port = atoi_results_u8; if (cmd_compare(1, "status")) { bandwidth_status(port); @@ -1688,12 +1658,9 @@ void cmd_parser(void) __banked stpEnabled = 0; } } else if (cmd_compare(0, "pvid") && cmd_words_len == 3) { - uint8_t ret; - if (cmd_buffer[cmd_words_b[1]] >= '1' - && cmd_buffer[cmd_words_b[1]] <= '9' - && cmd_buffer[cmd_words_b[1] + 1] <= ' ' + if (cmd_parse_port_space(cmd_words_b[1], false) != 0 && atoi_results_u8 && atoi_short(cmd_words_b[2]) && atoi_results_short && atoi_results_short <= 4094) - port_pvid_set(machine.phys_to_log_port[cmd_buffer[cmd_words_b[1]] - '1'], atoi_results_short); + port_pvid_set(atoi_results_u8, atoi_results_short); else print_string("Error: pvid <1-4094>\n"); } else if (cmd_compare(0, "vlan")) { From 3eb2dfe0c9151dba53b5b9b886f93c5b46318c52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Tue, 11 Aug 2026 23:44:32 +0200 Subject: [PATCH 07/26] Change parse_ip() --- cmd_parser.c | 70 ++++++++++++++++++++++++++++++---------------------- 1 file changed, 41 insertions(+), 29 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index 296741b..98ebc61 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -283,33 +283,43 @@ __bit cmd_is_space(uint8_t idx) { return cmd_buffer[idx] == ' '; } - -int8_t parse_ip(uint8_t idx) +// returns 0 when on parser error or invalid value or no space. +// return non-zero number of bytes consumed including the space. +// Stops at a space or NULL. +uint8_t parse_ip(uint8_t idx) { uint8_t b = 0; uint8_t ret; + uint8_t idx_start = idx; while(1) { ret = atoi_byte(idx); - if (ret == 0) { + if (ret == 0) goto err; - } + idx += ret; ip[b++] = atoi_results_u8; + ret = cmd_buffer[idx]; if (b == 4) { - break; - } - - if (cmd_buffer[idx++] != '.') { + if (ret == ' ') { + idx++; + break; + } + if (ret == '\0') + break; goto err; } + idx++; + + if (ret != '.') + goto err; } - return 0; + return idx - idx_start; err: print_string("Error in IP format\n"); - return -1; + return 0; } @@ -1388,7 +1398,7 @@ void parse_syslog(void) itoa(syslog_state.server_ip[0]); write_char('.'); itoa(syslog_state.server_ip[1]); write_char('.'); itoa(syslog_state.server_ip[2]); write_char('.'); itoa(syslog_state.server_ip[3]); return; - } else if (!parse_ip(cmd_words_b[2])) { + } else if (parse_ip(cmd_words_b[2]) != 0) { uint8_t was_enabled = syslog_state.enabled; if (was_enabled) syslog_stop(); @@ -1568,46 +1578,48 @@ void cmd_parser(void) __banked } else { if (dhcp_state.state) dhcp_stop(); - if (!parse_ip(cmd_words_b[1])) { + if (parse_ip(cmd_words_b[1]) != 0) { uip_ipaddr(&uip_hostaddr, ip[0], ip[1], ip[2], ip[3]); print_string("Setting ip: "); itoa(ip[0]); write_char('.'); itoa(ip[1]); write_char('.'); itoa(ip[2]); write_char('.'); itoa(ip[3]); write_char('\n'); } else { - print_string("Invalid IP address\n"); - print_string("Error: ip [|dhcp]\n"); - print_string(" The dhcp option enables the dhcp client, calling ip without options prints the current IP\n"); - print_string(" Calling with a valid IP address will stop any ongoing dhcp client and set the IP address\n"); + print_string("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"); } } } else if (cmd_compare(0, "gw")) { if (cmd_words_len == 1) { print_string("Current gw: "); itoa(uip_draddr[0]); write_char('.'); itoa(uip_draddr[0] >> 8); write_char('.'); - itoa(uip_draddr[1]); write_char('.'); itoa(uip_draddr[1] >> 8); + itoa(uip_draddr[1]); write_char('.'); itoa(uip_draddr[1] >> 8); write_char('\n'); } else { - if (!parse_ip(cmd_words_b[1])) + if (parse_ip(cmd_words_b[1]) != 0) { uip_ipaddr(&uip_draddr, ip[0], ip[1], ip[2], ip[3]); - else - print_string("Invalid IP address\n"); - print_string("Setting gw: "); - itoa(ip[0]); write_char('.'); itoa(ip[1]); write_char('.'); - itoa(ip[2]); write_char('.'); itoa(ip[3]); + print_string("Setting gw: "); + itoa(ip[0]); write_char('.'); itoa(ip[1]); write_char('.'); + itoa(ip[2]); write_char('.'); itoa(ip[3]); write_char('\n'); + } else { + print_string("Invalid IP address\n" \ + "Error: gw \n"); + } } - write_char('\n'); } else if (cmd_compare(0, "netmask")) { if (cmd_words_len == 1) { print_string("Current netmask: "); itoa(uip_netmask[0]); write_char('.'); itoa(uip_netmask[0] >> 8); write_char('.'); itoa(uip_netmask[1]); write_char('.'); itoa(uip_netmask[1] >> 8); } else { - if (!parse_ip(cmd_words_b[1])) + if (parse_ip(cmd_words_b[1]) != 0) { uip_ipaddr(&uip_netmask, ip[0], ip[1], ip[2], ip[3]); - else + print_string("Setting netmask: "); + itoa(ip[0]); write_char('.'); itoa(ip[1]); write_char('.'); + itoa(ip[2]); write_char('.'); itoa(ip[3]); + } else { print_string("Invalid IP address\n"); - print_string("Setting netmask: "); - itoa(ip[0]); write_char('.'); itoa(ip[1]); write_char('.'); - itoa(ip[2]); write_char('.'); itoa(ip[3]); + } } write_char('\n'); } else if (cmd_compare(0, "l2")) { From 724fd1e061b32edb7522cdb13f04840435d830d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Wed, 12 Aug 2026 01:08:47 +0200 Subject: [PATCH 08/26] Added print_ip() to print IPv4 addresses. --- cmd_parser.c | 40 ++++++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index 98ebc61..ef0bf04 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -322,6 +322,22 @@ err: return 0; } +// Print a IPv4 adress. +void print_ip(__xdata uint8_t * ptr) +{ + uint8_t idx = 0; + uint8_t num; + + while(1) { + num = *ptr++; + itoa(num); + if (++idx == 4) + break; + + write_char('.'); + } +} + void parse_lag(void) { @@ -1379,8 +1395,7 @@ void parse_syslog(void) print_string("Current syslog status: "); if (syslog_state.enabled) { print_string("enabled, sending to "); - itoa(syslog_state.server_ip[0]); write_char('.'); itoa(syslog_state.server_ip[1]); write_char('.'); - itoa(syslog_state.server_ip[2]); write_char('.'); itoa(syslog_state.server_ip[3]); + print_ip(syslog_state.server_ip); write_char('\n'); } else { print_string("disabled\n"); @@ -1395,8 +1410,7 @@ void parse_syslog(void) } else if (cmd_compare(1, "ip")) { if (cmd_words_len < 3) { // no additional arguemnt -> print current ip print_string("Current syslog IP: "); - itoa(syslog_state.server_ip[0]); write_char('.'); itoa(syslog_state.server_ip[1]); write_char('.'); - itoa(syslog_state.server_ip[2]); write_char('.'); itoa(syslog_state.server_ip[3]); + print_ip(syslog_state.server_ip); return; } else if (parse_ip(cmd_words_b[2]) != 0) { uint8_t was_enabled = syslog_state.enabled; @@ -1565,8 +1579,7 @@ void cmd_parser(void) __banked dhcp_start(); } else if (cmd_words_len == 1) { print_string("Current IP: "); - itoa(uip_hostaddr[0]); write_char('.'); itoa(uip_hostaddr[0] >> 8); write_char('.'); - itoa(uip_hostaddr[1]); write_char('.'); itoa(uip_hostaddr[1] >> 8); + print_ip(uip_hostaddr); if (dhcp_state.state == DHCP_LEASING) { print_string(" (dhcp, renewal in sec: "); print_short(dhcp_state.dhcp_timer); @@ -1581,8 +1594,7 @@ void cmd_parser(void) __banked if (parse_ip(cmd_words_b[1]) != 0) { uip_ipaddr(&uip_hostaddr, ip[0], ip[1], ip[2], ip[3]); print_string("Setting ip: "); - itoa(ip[0]); write_char('.'); itoa(ip[1]); write_char('.'); - itoa(ip[2]); write_char('.'); itoa(ip[3]); write_char('\n'); + print_ip(ip); write_char('\n'); } else { print_string("Invalid IP address\n" \ "Error: ip [|dhcp]\n" \ @@ -1593,14 +1605,12 @@ void cmd_parser(void) __banked } else if (cmd_compare(0, "gw")) { if (cmd_words_len == 1) { print_string("Current gw: "); - itoa(uip_draddr[0]); write_char('.'); itoa(uip_draddr[0] >> 8); write_char('.'); - itoa(uip_draddr[1]); write_char('.'); itoa(uip_draddr[1] >> 8); write_char('\n'); + print_ip(uip_draddr); write_char('\n'); } else { if (parse_ip(cmd_words_b[1]) != 0) { uip_ipaddr(&uip_draddr, ip[0], ip[1], ip[2], ip[3]); print_string("Setting gw: "); - itoa(ip[0]); write_char('.'); itoa(ip[1]); write_char('.'); - itoa(ip[2]); write_char('.'); itoa(ip[3]); write_char('\n'); + print_ip(ip); write_char('\n'); } else { print_string("Invalid IP address\n" \ "Error: gw \n"); @@ -1609,14 +1619,12 @@ void cmd_parser(void) __banked } else if (cmd_compare(0, "netmask")) { if (cmd_words_len == 1) { print_string("Current netmask: "); - itoa(uip_netmask[0]); write_char('.'); itoa(uip_netmask[0] >> 8); write_char('.'); - itoa(uip_netmask[1]); write_char('.'); itoa(uip_netmask[1] >> 8); + print_ip(uip_netmask); write_char('\n'); } else { if (parse_ip(cmd_words_b[1]) != 0) { uip_ipaddr(&uip_netmask, ip[0], ip[1], ip[2], ip[3]); print_string("Setting netmask: "); - itoa(ip[0]); write_char('.'); itoa(ip[1]); write_char('.'); - itoa(ip[2]); write_char('.'); itoa(ip[3]); + print_ip(ip); write_char('\n'); } else { print_string("Invalid IP address\n"); } From 17642699d4a3e666bda72e91b71b9a295dd4d4c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Fri, 14 Aug 2026 19:56:23 +0200 Subject: [PATCH 09/26] Improve parse_ingress() --- cmd_parser.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index ef0bf04..2a78936 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -283,6 +283,12 @@ __bit cmd_is_space(uint8_t idx) { return cmd_buffer[idx] == ' '; } +// check if the cmd_buffer[idx] is a space or null. +__bit cmd_is_space_or_null(uint8_t idx) { + uint8_t c = cmd_buffer[idx]; + return c == ' ' || c == '\0'; +} + // returns 0 when on parser error or invalid value or no space. // return non-zero number of bytes consumed including the space. // Stops at a space or NULL. @@ -598,10 +604,13 @@ void parse_ingress(void) if (cmd_words_len < 2) { goto err; } - __xdata uint8_t log_port = 0; + uint8_t log_port = 0; __xdata vlan_ingress_mode_t mode = VLAN_INVALID; + uint8_t idx = cmd_words_b[1]; - if (vlan_ingress_mode_parse(cmd_buffer[cmd_words_b[1]], &mode)) { + if (vlan_ingress_mode_parse(cmd_buffer[idx++], &mode)) { + if (!cmd_is_space_or_null(idx)) + goto err; // 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)) { @@ -613,17 +622,17 @@ void parse_ingress(void) } } else { for(uint8_t w = 1; w < cmd_words_len; w++) { - uint8_t idx = cmd_words_b[w]; + idx = cmd_words_b[w]; char p = cmd_buffer[idx]; - uint8_t ret = cmd_parse_port_space(idx, false); - if (ret == 0) { + uint8_t ret = cmd_parse_port(idx, false); + if (ret != 1) { print_string("Invalid physical port number\n"); continue; } log_port = atoi_results_u8; idx += ret; - if (!vlan_ingress_mode_parse(cmd_buffer[idx], &mode)) { + if (!vlan_ingress_mode_parse(cmd_buffer[idx++], &mode) || !cmd_is_space_or_null(idx)) { print_string("Invalid ingress mode for port "); write_char(p); print_string(" in ingress command\n"); goto err; } @@ -638,7 +647,7 @@ void parse_ingress(void) } return; err: - print_string("Error: ingress [p]... \n"); + print_string("Error: ingress [p]...\n"); } void parse_mirror(void) From 42f7b4fba1bc396607ba5159f8233c9e65e95d23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Fri, 14 Aug 2026 22:06:19 +0200 Subject: [PATCH 10/26] Fix parse_isolate(), ensure spaces between arguments --- cmd_parser.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index 2a78936..59fca82 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -529,7 +529,7 @@ err: void parse_isolate(void) { - __xdata uint16_t members = 0; + uint16_t members = 0; if (cmd_words_len < 3) goto err; @@ -555,7 +555,7 @@ void parse_isolate(void) members >>= 1; } return; - } + } if (cmd_compare(2, "off")) { for (uint8_t i = machine.min_port; i < machine.max_port; i++) @@ -567,7 +567,10 @@ void parse_isolate(void) uint8_t w = 2; while (w < cmd_words_len) { - if (cmd_parse_port(cmd_words_b[w++], false) == 0) + uint8_t idx = cmd_words_b[w++]; + uint8_t ret = cmd_parse_port(idx, false); + idx += ret; + if (ret == 0 || !cmd_is_space_or_null(idx)) goto err; uint8_t port = atoi_results_u8; members |= ((uint16_t)1) << port; From 149dc8537ffc2dbd50ab7cec1e24fa8136e9baf3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Mon, 17 Aug 2026 22:37:41 +0200 Subject: [PATCH 11/26] Rename cmd_parse_port_space() to cmd_parse_port_separator() cmd_parse_port_separator() parse the full port number and checks that the number end with a NUL of space. Now this function can also be used to parse the last argument number because this ends with a NUL. Also refactor code. --- cmd_parser.c | 46 +++++++++++++++++++++++++++------------------- 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index 59fca82..0d85aa7 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -264,14 +264,15 @@ uint8_t cmd_parse_port(uint8_t idx, __bit cpu_is_valid) { // Same as cmd_parse_port() addition to check the trailing space. -// returns 0 when on parser error or invalid value or no space. +// returns 0 when on parser error or invalid value or no space or no NUL. // return non-zero number of bytes consumed including the space. -uint8_t cmd_parse_port_space(uint8_t idx, __bit cpu_is_valid) { +uint8_t cmd_parse_port_separator(uint8_t idx, __bit cpu_is_valid) { uint8_t ret = cmd_parse_port(idx, cpu_is_valid); if (ret != 0) { idx += ret; ret++; - if (cmd_buffer[idx] != ' ') + uint8_t c = cmd_buffer[idx]; + if (c != ' ' && c != '\0') ret = 0; } return ret; @@ -385,7 +386,7 @@ void parse_lag(void) goto err; group = atoi_results_u8 - 1; - if (group > 3) /* '0' wraps well past three, so one test does both ends */ + if (group > 3) goto err; uint8_t w = 2; @@ -393,7 +394,7 @@ void parse_lag(void) // write_char('|'); print_byte(w); write_char(':'); write_char(cmd_buffer[cmd_words_b[w]]); write_char('-'); // Parse port. - if (cmd_parse_port(cmd_words_b[w++], false) == 0) + if (cmd_parse_port_separator(cmd_words_b[w++], false) == 0) goto err; members |= ((uint16_t)1) << atoi_results_u8; @@ -407,13 +408,16 @@ err: void parse_lag_hash(void) { - __xdata uint8_t group; __xdata uint8_t hash = 0; - if (cmd_words_len < 2 || !isnumber(cmd_buffer[cmd_words_b[1]])) + if (cmd_words_len < 2) goto err; - group = cmd_buffer[cmd_words_b[1]] - '1'; - if (group > 3) /* '0' wraps well past three, so one test does both ends */ + + if (cmd_parse_port_separator(cmd_buffer[cmd_words_b[1]], false) == 0) + goto err; + + uint8_t group = atoi_results_u8 - 1; + if (group > 3) goto err; uint8_t w = 2; @@ -507,8 +511,13 @@ void parse_vlan(void) uint16_t pmask = ((uint16_t)1) << atoi_results_u8; vlan_settings.members |= pmask; - if (cmd_buffer[idx] == 't') + if (cmd_buffer[idx] == 't') { vlan_settings.tagged |= pmask; + idx++; + } + + if (!cmd_is_space_or_null(idx)) + goto err; } vlan_create(); } else if (cmd_compare(1, "show")) { @@ -536,7 +545,7 @@ void parse_isolate(void) print_string("\nISOLATE "); - if (cmd_parse_port_space(cmd_words_b[1], false) == 0) + if (cmd_parse_port_separator(cmd_words_b[1], false) == 0) goto err; uint8_t port_configured = atoi_results_u8; @@ -568,9 +577,8 @@ void parse_isolate(void) uint8_t w = 2; while (w < cmd_words_len) { uint8_t idx = cmd_words_b[w++]; - uint8_t ret = cmd_parse_port(idx, false); - idx += ret; - if (ret == 0 || !cmd_is_space_or_null(idx)) + uint8_t ret = cmd_parse_port_separator(idx, false); + if (ret == 0) goto err; uint8_t port = atoi_results_u8; members |= ((uint16_t)1) << port; @@ -730,7 +738,7 @@ void parse_port(void) return; } - if (cmd_parse_port_space(cmd_words_b[1], false) == 0) { + if (cmd_parse_port_separator(cmd_words_b[1], false) == 0) { print_string("Invalid port number\n"); return; } @@ -830,7 +838,7 @@ void parse_mtu(void) if (cmd_words_len != 3) goto err; - if (cmd_parse_port_space(cmd_words_b[1], false) == 0) + if (cmd_parse_port_separator(cmd_words_b[1], false) == 0) goto err; p = atoi_results_u8; @@ -1284,7 +1292,7 @@ void parse_eee(void) speed_word = 2; } else if (cmd_buffer[idx] == ' ' || cmd_buffer[idx] == '\0') { // Word 2 is a port number - if (cmd_parse_port_space(idx, false) == 0) { + if (cmd_parse_port_separator(idx, false) == 0) { print_string("Speed word invalid, use: [100m|1g|2g5]\n"); return; } @@ -1337,7 +1345,7 @@ void parse_bw(void) if (cmd_words_len < 2) // Check for at least 2 arguments goto err; - if (cmd_parse_port_space(cmd_words_b[2], false) == 0) + if (cmd_parse_port_separator(cmd_words_b[2], false) == 0) goto err; port = atoi_results_u8; @@ -1690,7 +1698,7 @@ void cmd_parser(void) __banked stpEnabled = 0; } } else if (cmd_compare(0, "pvid") && cmd_words_len == 3) { - if (cmd_parse_port_space(cmd_words_b[1], false) != 0 && atoi_results_u8 + if (cmd_parse_port_separator(cmd_words_b[1], false) != 0 && atoi_results_u8 && atoi_short(cmd_words_b[2]) && atoi_results_short && atoi_results_short <= 4094) port_pvid_set(atoi_results_u8, atoi_results_short); else From dca92d7f45c785591ee54b838c9804e5edc3990d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Mon, 17 Aug 2026 22:53:27 +0200 Subject: [PATCH 12/26] cmd_parse_port_separator() cmd_parse_port() remove CPU-port support Removing the CPU-port support, As stated in #334, it is not needed to manual add the CPU-port to any command. Because the CPU-port should be added automatilly when CPU-port is needed. Refactor cmd_parse_port() because port number is only 1 byte. This simplifies the parsing code. Refactor cmd_parse_port_separator() to ensure the return size is correct. --- cmd_parser.c | 69 ++++++++++++++++++++++------------------------------ 1 file changed, 29 insertions(+), 40 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index 0d85aa7..05f1f26 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -234,45 +234,37 @@ uint8_t atoi_short(uint8_t idx) return cnt; } -/* Parse, validate and translate phys_to_log_port port argument - * cpu_is_valid tells the parser that port 10 is valid input. +/* Parse, validate and translate phys_to_log_port physical port argument. + * So CPU-port 10 is not valid. * returns 0 when on parser error or invalid value. * return non-zero number of bytes consumed. * Store the value in atoi_results_u8. */ -uint8_t cmd_parse_port(uint8_t idx, __bit cpu_is_valid) { - uint8_t ret = atoi_byte(idx); - uint8_t port = 0; - if (ret != 0) { - port = atoi_results_u8 - 1; +uint8_t cmd_parse_port(uint8_t idx) { + uint8_t port = cmd_buffer[idx] - '0' - 1; + if (port > 8) + return 0; - if (cpu_is_valid && port == 9) { - // CPU port is valid - } else if (port < 9) { - port = machine.phys_to_log_port[port]; - if (port < machine.min_port || port > machine.max_port) { - ret = 0; - } - } else { - ret = 0; - } - } + port = machine.phys_to_log_port[port]; + if (port < machine.min_port || port > machine.max_port) + return 0; atoi_results_u8 = port; - return ret; + return 1; } -// Same as cmd_parse_port() addition to check the trailing space. +// Same as cmd_parse_port() addition to check the trailing space or NUL. // returns 0 when on parser error or invalid value or no space or no NUL. // return non-zero number of bytes consumed including the space. -uint8_t cmd_parse_port_separator(uint8_t idx, __bit cpu_is_valid) { - uint8_t ret = cmd_parse_port(idx, cpu_is_valid); +uint8_t cmd_parse_port_separator(uint8_t idx) { + uint8_t ret = cmd_parse_port(idx); if (ret != 0) { idx += ret; - ret++; uint8_t c = cmd_buffer[idx]; - if (c != ' ' && c != '\0') + if (c == ' ') { + ret++; + } else if (c != '\0') ret = 0; } return ret; @@ -394,7 +386,7 @@ void parse_lag(void) // write_char('|'); print_byte(w); write_char(':'); write_char(cmd_buffer[cmd_words_b[w]]); write_char('-'); // Parse port. - if (cmd_parse_port_separator(cmd_words_b[w++], false) == 0) + if (cmd_parse_port_separator(cmd_words_b[w++]) == 0) goto err; members |= ((uint16_t)1) << atoi_results_u8; @@ -413,7 +405,7 @@ void parse_lag_hash(void) if (cmd_words_len < 2) goto err; - if (cmd_parse_port_separator(cmd_buffer[cmd_words_b[1]], false) == 0) + if (cmd_parse_port_separator(cmd_buffer[cmd_words_b[1]]) == 0) goto err; uint8_t group = atoi_results_u8 - 1; @@ -503,7 +495,7 @@ void parse_vlan(void) uint8_t idx; while (cmd_words_len > w) { idx = cmd_words_b[w++]; - ret = cmd_parse_port(idx, false); + ret = cmd_parse_port(idx); if (ret == 0) goto err; @@ -545,7 +537,7 @@ void parse_isolate(void) print_string("\nISOLATE "); - if (cmd_parse_port_separator(cmd_words_b[1], false) == 0) + if (cmd_parse_port_separator(cmd_words_b[1]) == 0) goto err; uint8_t port_configured = atoi_results_u8; @@ -576,9 +568,7 @@ void parse_isolate(void) uint8_t w = 2; while (w < cmd_words_len) { - uint8_t idx = cmd_words_b[w++]; - uint8_t ret = cmd_parse_port_separator(idx, false); - if (ret == 0) + if (cmd_parse_port_separator(cmd_words_b[w++]) == 0) goto err; uint8_t port = atoi_results_u8; members |= ((uint16_t)1) << port; @@ -635,7 +625,7 @@ void parse_ingress(void) for(uint8_t w = 1; w < cmd_words_len; w++) { idx = cmd_words_b[w]; char p = cmd_buffer[idx]; - uint8_t ret = cmd_parse_port(idx, false); + uint8_t ret = cmd_parse_port(idx); if (ret != 1) { print_string("Invalid physical port number\n"); continue; @@ -701,7 +691,7 @@ void parse_mirror(void) uint8_t ret; while (w < cmd_words_len) { uint8_t idx = cmd_words_b[w++]; - ret = cmd_parse_port(idx, false); + ret = cmd_parse_port(idx); if (ret == 0) goto err; @@ -738,7 +728,7 @@ void parse_port(void) return; } - if (cmd_parse_port_separator(cmd_words_b[1], false) == 0) { + if (cmd_parse_port_separator(cmd_words_b[1]) == 0) { print_string("Invalid port number\n"); return; } @@ -838,7 +828,7 @@ void parse_mtu(void) if (cmd_words_len != 3) goto err; - if (cmd_parse_port_separator(cmd_words_b[1], false) == 0) + if (cmd_parse_port_separator(cmd_words_b[1]) == 0) goto err; p = atoi_results_u8; @@ -1292,7 +1282,7 @@ void parse_eee(void) speed_word = 2; } else if (cmd_buffer[idx] == ' ' || cmd_buffer[idx] == '\0') { // Word 2 is a port number - if (cmd_parse_port_separator(idx, false) == 0) { + if (cmd_parse_port_separator(idx) == 0) { print_string("Speed word invalid, use: [100m|1g|2g5]\n"); return; } @@ -1339,15 +1329,14 @@ void parse_eee(void) void parse_bw(void) { - uint8_t port; __xdata uint32_t bw = 0; if (cmd_words_len < 2) // Check for at least 2 arguments goto err; - if (cmd_parse_port_separator(cmd_words_b[2], false) == 0) + if (cmd_parse_port_separator(cmd_words_b[2]) == 0) goto err; - port = atoi_results_u8; + uint8_t port = atoi_results_u8; if (cmd_compare(1, "status")) { bandwidth_status(port); @@ -1698,7 +1687,7 @@ void cmd_parser(void) __banked stpEnabled = 0; } } else if (cmd_compare(0, "pvid") && cmd_words_len == 3) { - if (cmd_parse_port_separator(cmd_words_b[1], false) != 0 && atoi_results_u8 + if (cmd_parse_port_separator(cmd_words_b[1]) != 0 && atoi_results_u8 && atoi_short(cmd_words_b[2]) && atoi_results_short && atoi_results_short <= 4094) port_pvid_set(atoi_results_u8, atoi_results_short); else From f62755900519eb2affb9ab4b624d0bc89da220c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Tue, 18 Aug 2026 18:53:28 +0200 Subject: [PATCH 13/26] fix CMD: pvid: remove atoi_results_u8 from check --- cmd_parser.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index 05f1f26..fe33db0 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -1686,8 +1686,8 @@ void cmd_parser(void) __banked stp_off(); stpEnabled = 0; } - } else if (cmd_compare(0, "pvid") && cmd_words_len == 3) { - if (cmd_parse_port_separator(cmd_words_b[1]) != 0 && atoi_results_u8 + } else if (cmd_compare(0, "pvid")) { + 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 From 5ddec9710ad61a027f326e3f72bd06fe09b73417 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Tue, 18 Aug 2026 19:07:13 +0200 Subject: [PATCH 14/26] CMD: Fix parse_lag_hash() --- cmd_parser.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index fe33db0..83bae49 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -400,18 +400,15 @@ err: void parse_lag_hash(void) { - __xdata uint8_t hash = 0; - - if (cmd_words_len < 2) - goto err; - - if (cmd_parse_port_separator(cmd_buffer[cmd_words_b[1]]) == 0) + // Parse group, expect only one number 0-9. + if (cmd_words_len < 3 || atoi_byte(cmd_words_b[1]) != 1) goto err; uint8_t group = atoi_results_u8 - 1; if (group > 3) goto err; + uint8_t hash = 0; uint8_t w = 2; while (w < cmd_words_len) { if (cmd_compare(w, "spa")) @@ -432,13 +429,14 @@ void parse_lag_hash(void) print_string("Error: invalid hash type:"); print_string_x(&cmd_buffer[cmd_words_b[w]]); write_char('\n'); + goto err; } w++; } port_lag_hash_set(group, hash); return; err: - print_string("Error: lag hash <1-4> [type]...\n"); + print_string("Error: laghash <1-4> [smac|dmac|sip|dip|sport|dport]\n"); } From 3776f12f5c986c2a05cdb10c10ee2d4f73b2579a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Tue, 18 Aug 2026 20:11:59 +0200 Subject: [PATCH 15/26] Fix and refactor parse_bw() --- cmd_parser.c | 37 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index 83bae49..fe76721 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -1327,9 +1327,7 @@ void parse_eee(void) void parse_bw(void) { - __xdata uint32_t bw = 0; - - if (cmd_words_len < 2) // Check for at least 2 arguments + if (cmd_words_len < 3) // Check for at least 3 arguments goto err; if (cmd_parse_port_separator(cmd_words_b[2]) == 0) @@ -1344,8 +1342,13 @@ void parse_bw(void) if (cmd_words_len < 4) // Check for at least 4 arguments goto err; + // Ensure first argument is only `in` or `out`. + __bit is_in = cmd_compare(1, "in") != 0; + if (!(is_in || cmd_compare(1, "out"))) + goto err; + if (cmd_compare(3, "drop")) { - if (cmd_compare(1, "in")) { + if (is_in) { bandwidth_ingress_drop(port); return; } @@ -1353,7 +1356,7 @@ void parse_bw(void) } if (cmd_compare(3, "fc")) { - if (cmd_compare(1, "in")) { + if (is_in) { bandwidth_ingress_fc(port); return; } @@ -1361,34 +1364,30 @@ void parse_bw(void) } if (cmd_compare(3, "off")) { - if (cmd_compare(1, "in")) { + if (is_in) { bandwidth_ingress_disable(port); - return; - } else if (cmd_compare(1, "out")) { + } else { bandwidth_egress_disable(port); - return; } - goto err; + return; } + __xdata uint32_t bw = 0; uint8_t hex_size = atoi_hex(cmd_words_b[3]); - if (hex_size == 0 || hex_size > 4) { + if (hex_size == 0 || hex_size > 4) goto err; - } + uint8_t i = 0; - while (hex_size) { + do { hex_size--; *(((uint8_t *) &bw) + hex_size) = hexvalue[i++]; - } + } while (hex_size); - if (cmd_compare(1, "in")) { + if (is_in) { bandwidth_ingress_set(port, bw); - } else if (cmd_compare(1, "out")) { - bandwidth_egress_set(port, bw); } else { - goto err; + bandwidth_egress_set(port, bw); } - return; err: From 3c183f73f8e72f28cb68d836d9f3a0832b107694 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Tue, 18 Aug 2026 20:55:57 +0200 Subject: [PATCH 16/26] Added print_port() helper to print physical port number. --- cmd_parser.c | 21 ++++++++------------- rtl837x_common.h | 1 + rtl837x_phy.c | 4 ++-- rtl837x_port.c | 21 ++++----------------- rtlplayground.c | 13 +++++++++++++ 5 files changed, 28 insertions(+), 32 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index fe76721..c3582d7 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -27,7 +27,6 @@ extern __code struct machine machine; extern __xdata uint8_t stpEnabled; -extern __code uint8_t log_to_phys_port[9]; extern volatile __xdata uint32_t ticks; extern volatile __xdata uint8_t sfr_data[4]; @@ -356,7 +355,7 @@ void parse_lag(void) print_string(" member ports: "); for (uint8_t j = 0; j < 10; j++) { if (members & 1) { - write_char('0' + machine.log_to_phys_port[j]); + print_port(j); write_char(' '); } members >>= 1; @@ -545,10 +544,7 @@ void parse_isolate(void) members = port_isolation_get(port_configured); for (uint8_t i = 0; i < 10; i++) { if (members & 1) { - if (i < 9) - write_char(machine.log_to_phys_port[i] + '0'); - else - print_string("CPU"); + print_port(i); write_char(' '); } members >>= 1; @@ -613,7 +609,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)) { - print_string("Error setting ingress filter for port "); print_byte(machine.log_to_phys_port[log_port]); write_char('\n'); + print_string("Error setting ingress filter for port "); print_port(log_port); write_char('\n'); return; } print_string("All ports ingress filter set to: "); @@ -622,7 +618,6 @@ void parse_ingress(void) } else { for(uint8_t w = 1; w < cmd_words_len; w++) { idx = cmd_words_b[w]; - char p = cmd_buffer[idx]; uint8_t ret = cmd_parse_port(idx); if (ret != 1) { print_string("Invalid physical port number\n"); @@ -632,14 +627,14 @@ void parse_ingress(void) idx += ret; if (!vlan_ingress_mode_parse(cmd_buffer[idx++], &mode) || !cmd_is_space_or_null(idx)) { - print_string("Invalid ingress mode for port "); write_char(p); print_string(" in ingress command\n"); + print_string("Invalid ingress mode for port "); print_port(log_port); print_string(" in ingress command\n"); goto err; } if (!port_ingress_filter(log_port, mode)) { - print_string("Error setting ingress filter for port "); write_char(p); write_char('\n'); + print_string("Error setting ingress filter for port "); print_port(log_port); write_char('\n'); return; } - print_string("Port "); write_char(p); + print_string("Port "); print_port(log_port); print_string(" ingress filter set to: "); print_port_ingress_filter_mode(mode); write_char('\n'); } @@ -664,7 +659,7 @@ void parse_mirror(void) print_string("NOT Enabled: "); } print_string("Mirroring port: "); - write_char('0' + machine.log_to_phys_port[mPort >> 1]); + print_port(mirroring_port); reg_read_m(RTL837x_MIRROR_CONF); uint16_t m = sfr_data[0]; m = (m << 8) | sfr_data[1]; @@ -818,7 +813,7 @@ void parse_mtu(void) for (p = machine.min_port; p <= machine.max_port; p++) { reg_read_m(RTL8373_REG_MAC_L2_PORT_MAX_LEN + ((uint16_t) p << 8)); uint16_t mtu = SFR_DATA_U16 & 0x3fff; - print_string("Port "); print_byte(machine.log_to_phys_port[p]); + print_string("Port "); print_port(p); write_char(' '); print_short(mtu); write_char('\n'); } return; diff --git a/rtl837x_common.h b/rtl837x_common.h index 861c1e6..27d9449 100644 --- a/rtl837x_common.h +++ b/rtl837x_common.h @@ -135,6 +135,7 @@ void itoa(uint8_t v); void print_sfr_data(void); void print_phy_data(void); void print_cmd_prompt(void); +void print_port(uint8_t port); void phy_write_mask(uint16_t phy_mask, uint8_t dev_id, uint16_t reg, uint16_t v); void phy_write(uint8_t phy_id, uint8_t dev_id, uint16_t reg, uint16_t v); void phy_read(uint8_t phy_id, uint8_t dev_id, uint16_t reg); diff --git a/rtl837x_phy.c b/rtl837x_phy.c index 16fd214..5512755 100644 --- a/rtl837x_phy.c +++ b/rtl837x_phy.c @@ -262,7 +262,7 @@ void phy_set_speed(void) __banked { uint16_t v; - print_string("Setting port "); write_char(machine.log_to_phys_port[phy_settings.port] + '0'); + print_string("Setting port "); print_port(phy_settings.port); if (machine.n_10g && phy_settings.port == 3) phy_settings.is10g_port = 1; if (machine.n_10g == 2 && phy_settings.port == 8) @@ -381,7 +381,7 @@ void phy_set_duplex(void) __banked { uint16_t v; - print_string("Setting port "); write_char(machine.log_to_phys_port[phy_settings.port] + '0'); + print_string("Setting port "); print_port(phy_settings.port); if (phy_settings.duplex) print_string(" to full duplex"); else diff --git a/rtl837x_port.c b/rtl837x_port.c index a85e90d..bc073d0 100644 --- a/rtl837x_port.c +++ b/rtl837x_port.c @@ -390,10 +390,7 @@ void port_l2_learned(void) __banked print_string("\tlearned\t"); port |= (sfr_data[3] & 0x3) << 2; - if (port < 9) - write_char(machine.log_to_phys_port[port] + '0'); - else - print_string("CPU"); + print_port(port); } entry++; @@ -431,7 +428,7 @@ void port_stats_print(void) __banked { print_string("\nPort\tState\tLink\tTxGood\t\tTxBad\t\tRxGood\t\tRxBad\n"); for (uint8_t i = machine.min_port; i <= machine.max_port; i++) { - write_char('0' + machine.log_to_phys_port[i]); write_char('\t'); + print_port(i); write_char('\t'); if (!machine.is_sfp[i]) { phy_read(i, PHY_MMD31, 0xa610); @@ -606,7 +603,7 @@ void port_eee_disable(uint8_t port) __banked void port_eee_status(uint8_t port) __banked { - print_string("Port: "); write_char('0' + machine.log_to_phys_port[port]); + print_string("Port: "); print_port(port); print_string(": "); if (machine.is_sfp[port]) { print_string("SFP\n"); @@ -801,19 +798,9 @@ void print_port_ingress_filter_mode(vlan_ingress_mode_t mode) __banked } } -static void print_phys_port(uint8_t port) __banked -{ - if (port >= machine.min_port && port <= machine.max_port) - write_char(machine.log_to_phys_port[port] + '0'); - else if (port == 9) - write_char('9'); - else - write_char('?'); -} - void print_vlan_ingress_port(uint8_t log_port) __banked { - print_phys_port(log_port);write_char('\t'); + print_port(log_port);write_char('\t'); print_short(port_pvid_get(log_port));write_char('\t'); print_port_ingress_filter_mode(port_ingress_filter_get(log_port));write_char('\t'); port_ingress_vlan_filter_get(log_port) ? print_string("Enabled") : print_string("Disabled"); diff --git a/rtlplayground.c b/rtlplayground.c index d7146e3..d94d0fb 100644 --- a/rtlplayground.c +++ b/rtlplayground.c @@ -813,6 +813,19 @@ void print_reg(uint16_t reg) print_sfr_data(); } +// Print the phy port for a log port number. +void print_port(uint8_t port) +{ + if (port < 9) + write_char(machine.log_to_phys_port[port] + '0'); + else if (port == 9) + print_string("CPU"); + else { + print_string("UNKNOWN "); + write_char(port + '0'); + } +} + /* // TODO: This uses 2 DSEG bytes and is not used! From 5d42e8843ef28068e56e3145447266a674a2e91e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Tue, 18 Aug 2026 22:05:26 +0200 Subject: [PATCH 17/26] Rename cmd_is_space_or_null to cmd_is_space_or_nul --- cmd_parser.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index c3582d7..1f4da2d 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -275,15 +275,15 @@ __bit cmd_is_space(uint8_t idx) { return cmd_buffer[idx] == ' '; } -// check if the cmd_buffer[idx] is a space or null. -__bit cmd_is_space_or_null(uint8_t idx) { +// check if the cmd_buffer[idx] is a space or NULL. +__bit cmd_is_space_or_nul(uint8_t idx) { uint8_t c = cmd_buffer[idx]; return c == ' ' || c == '\0'; } // returns 0 when on parser error or invalid value or no space. // return non-zero number of bytes consumed including the space. -// Stops at a space or NULL. +// Stops at a space or NUL. uint8_t parse_ip(uint8_t idx) { uint8_t b = 0; @@ -479,7 +479,7 @@ void parse_vlan(void) vlan_names[vlan_ptr++] = hex[(vlan_settings.vlan >> 8) & 0xf]; vlan_names[vlan_ptr++] = hex[(vlan_settings.vlan >> 4) & 0xf] ; vlan_names[vlan_ptr++] = hex[vlan_settings.vlan & 0xf]; - while(cmd_buffer[cmd_words_b[w] + i] != ' ' && cmd_buffer[cmd_words_b[w] + i] != '\0') { + while(!cmd_is_space_or_nul(cmd_words_b[w] + i)) { write_char(cmd_buffer[cmd_words_b[w] + i]); vlan_names[vlan_ptr++] = cmd_buffer[cmd_words_b[w] + i++]; } @@ -505,7 +505,7 @@ void parse_vlan(void) idx++; } - if (!cmd_is_space_or_null(idx)) + if (!cmd_is_space_or_nul(idx)) goto err; } vlan_create(); @@ -604,7 +604,7 @@ void parse_ingress(void) uint8_t idx = cmd_words_b[1]; if (vlan_ingress_mode_parse(cmd_buffer[idx++], &mode)) { - if (!cmd_is_space_or_null(idx)) + if (!cmd_is_space_or_nul(idx)) goto err; // Setting mode for all ports at once for (log_port = machine.min_port; log_port <= machine.max_port; log_port++) { @@ -626,7 +626,7 @@ void parse_ingress(void) log_port = atoi_results_u8; idx += ret; - if (!vlan_ingress_mode_parse(cmd_buffer[idx++], &mode) || !cmd_is_space_or_null(idx)) { + if (!vlan_ingress_mode_parse(cmd_buffer[idx++], &mode) || !cmd_is_space_or_nul(idx)) { print_string("Invalid ingress mode for port "); print_port(log_port); print_string(" in ingress command\n"); goto err; } @@ -1273,7 +1273,7 @@ void parse_eee(void) if (cmd_buffer[idx] == 'g' || cmd_buffer[idx] == 'm') { // Word 2 is a speed (e.g., "2g5", "100m", "1g") speed_word = 2; - } else if (cmd_buffer[idx] == ' ' || cmd_buffer[idx] == '\0') { + } else if (cmd_is_space_or_nul(idx)) { // Word 2 is a port number if (cmd_parse_port_separator(idx) == 0) { print_string("Speed word invalid, use: [100m|1g|2g5]\n"); From 2ff418f80f936f1d46b41cb6bfe831aacc7fe6e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Tue, 18 Aug 2026 22:10:31 +0200 Subject: [PATCH 18/26] Replace '\0' to NUL to make it more clear that it is a NUL-terminated string. --- cmd_editor.c | 2 +- cmd_parser.c | 38 +++++++++++++++++++------------------- httpd/httpd.c | 20 ++++++++++---------- rtl837x_common.h | 1 + rtlplayground.c | 8 ++++---- 5 files changed, 35 insertions(+), 34 deletions(-) diff --git a/cmd_editor.c b/cmd_editor.c index 866cf40..aebfe7f 100644 --- a/cmd_editor.c +++ b/cmd_editor.c @@ -193,7 +193,7 @@ void cmd_edit(void) __banked // Check whether return was pressed: if (sbuf[l] == '\n' || sbuf[l] == '\r') { write_char('\n'); - cmd_buffer[cmd_line_len] = '\0'; + cmd_buffer[cmd_line_len] = NUL; // write_char('>'); print_string_x(cmd_buffer); write_char('<'); // If there is a command we print the prompt after execution // otherwise immediately because there is nothing to execute diff --git a/cmd_parser.c b/cmd_parser.c index 1f4da2d..d75233f 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -114,8 +114,8 @@ uint8_t cmd_compare(uint8_t start, __code uint8_t * cmd) uint8_t b = cmd_buffer[i]; // cmd is garanteerd to be NULL-terminated. - if (c == '\0') { - if ((b == ' ') || (b == '\0')) { + if (c == NUL) { + if ((b == ' ') || (b == NUL)) { // Match return 1; } @@ -147,7 +147,7 @@ uint8_t atoi_hex(uint8_t idx) while(1) { c = cmd_buffer[idx]; - if (c == '\0' || c == ' ') { + if (c == NUL || c == ' ') { break; } @@ -263,7 +263,7 @@ uint8_t cmd_parse_port_separator(uint8_t idx) { uint8_t c = cmd_buffer[idx]; if (c == ' ') { ret++; - } else if (c != '\0') + } else if (c != NUL) ret = 0; } return ret; @@ -278,7 +278,7 @@ __bit cmd_is_space(uint8_t idx) { // check if the cmd_buffer[idx] is a space or NULL. __bit cmd_is_space_or_nul(uint8_t idx) { uint8_t c = cmd_buffer[idx]; - return c == ' ' || c == '\0'; + return c == ' ' || c == NUL; } // returns 0 when on parser error or invalid value or no space. @@ -304,7 +304,7 @@ uint8_t parse_ip(uint8_t idx) idx++; break; } - if (ret == '\0') + if (ret == NUL) break; goto err; } @@ -483,7 +483,7 @@ void parse_vlan(void) write_char(cmd_buffer[cmd_words_b[w] + i]); vlan_names[vlan_ptr++] = cmd_buffer[cmd_words_b[w] + i++]; } - vlan_names[vlan_ptr++] = ' '; vlan_names[vlan_ptr] = '\0'; + vlan_names[vlan_ptr++] = ' '; vlan_names[vlan_ptr] = NUL; w++; print_string("<\n"); } @@ -738,11 +738,11 @@ void parse_port(void) } } else if (cmd_compare(2, "name")) { uint8_t i = 0; - while ( (i < PORT_NAME_SIZE-1) && (cmd_buffer[cmd_words_b[3] + i] != '\0') ) { + while ( (i < PORT_NAME_SIZE-1) && (cmd_buffer[cmd_words_b[3] + i] != NUL) ) { port_names[phy_settings.port][i] = cmd_buffer[cmd_words_b[3] + i]; i++; } - port_names[phy_settings.port][i] = '\0'; + port_names[phy_settings.port][i] = NUL; print_string("\nName set to: \""); print_string_x(port_names[phy_settings.port]); print_string("\"\n"); @@ -1248,8 +1248,8 @@ void parse_passwd(void) do { c = cmd_buffer[i++]; passwd[j++] = c; - } while (c != '\0' && j < 20); - passwd[j] = '\0'; + } while (c != NUL && j < 20); + passwd[j] = NUL; return; } print_string("Missing password\n"); @@ -1453,7 +1453,7 @@ void cmd_tokenize(void) __banked while(1) { c = cmd_buffer[line_ptr]; - if (c == '\0') { + if (c == NUL) { // Store the word count cmd_words_len = word; break; @@ -1658,13 +1658,13 @@ void cmd_parser(void) __banked __xdata char *dst = hostname; for (uint8_t hn = 0; hn < sizeof(hostname) - 1; hn++) { uint8_t c = *hp++; - if (c == '\0' || c == '\r' || c == '\n') + if (c == NUL || c == '\r' || c == '\n') break; if (c < 0x20 || c > 0x7e || c == '"' || c == '\\') c = '.'; *dst++ = c; } - *dst = '\0'; + *dst = NUL; } else { print_string("Error: hostname [name] - the name must not contain spaces\n"); } @@ -1752,7 +1752,7 @@ void cmd_parser(void) __banked uint8_t i = cmd_words_b[cmd_words_len - 1]; do { i++; - } while(cmd_buffer[i] != '\0'); + } while(cmd_buffer[i] != NUL); // Copy last cmd-buffer to history. cmd_history_ptr = (cmd_history_ptr + i) & CMD_HISTORY_MASK; @@ -1800,7 +1800,7 @@ void execute_config(void) __banked uint8_t c = 0; do { if (cmd_idx >= (CMD_BUF_SIZE - 1)) { - cmd_buffer[cmd_idx] = '\0'; + cmd_buffer[cmd_idx] = NUL; print_string("ERROR: Command too long: "); print_string_x(cmd_buffer); write_char('\n'); @@ -1809,7 +1809,7 @@ void execute_config(void) __banked } c = flash_buf[cfg_idx++]; if (c == 0 || c == '\n') { - cmd_buffer[cmd_idx] = '\0'; + cmd_buffer[cmd_idx] = NUL; if (cmd_idx) { cmd_tokenize(); if (err_status != ERR_OK) @@ -1845,7 +1845,7 @@ void execute_commands(__xdata uint8_t *p) __banked { while (1) { if (*p == 0 || *p == '\n' || *p == '\r') { if (cmd_idx) { - cmd_buffer[cmd_idx] = '\0'; + cmd_buffer[cmd_idx] = NUL; cmd_tokenize(); if (err_status != ERR_OK) return; @@ -1858,7 +1858,7 @@ void execute_commands(__xdata uint8_t *p) __banked { if (cmd_idx < (CMD_BUF_SIZE - 1)) { cmd_buffer[cmd_idx++] = *p; } else { - cmd_buffer[CMD_BUF_SIZE - 1] = '\0'; + cmd_buffer[CMD_BUF_SIZE - 1] = NUL; print_string("ERROR: Command too long: "); print_string_x(cmd_buffer); write_char('\n'); diff --git a/httpd/httpd.c b/httpd/httpd.c index ff4eff9..8f52631 100644 --- a/httpd/httpd.c +++ b/httpd/httpd.c @@ -119,8 +119,8 @@ bool is_word(__xdata uint8_t *xdata_str_p, __code uint8_t * __xdata code_str_p) u = *xdata_str_p++; c = *code_str_p++; - if (c == '\0') { - if (u != '\0' && u != ' ' && u != '\t' && u != ':' && u != '?' && u != '=' && u != '\n' && u != '\r') + if (c == NUL) { + if (u != NUL && u != ' ' && u != '\t' && u != ':' && u != '?' && u != '=' && u != '\n' && u != '\r') return false; return true; } @@ -140,8 +140,8 @@ bool is_url_word_x(__xdata uint8_t *uri_str_p, __xdata uint8_t *src_str_p) u = *uri_str_p++; s = *src_str_p++; - if (s == '\0') { - if (u != '\0' && u != ' ' && u != '\t' && u != ':' && u != '?' && u != '=' && u != '\n' && u != '\r') + if (s == NUL) { + if (u != NUL && u != ' ' && u != '\t' && u != ':' && u != '?' && u != '=' && u != '\n' && u != '\r') return false; return true; } @@ -183,9 +183,9 @@ bool is_word_x(__xdata uint8_t *lhs_str_p, __xdata uint8_t *rhs_str_p) u = *lhs_str_p++; c = *rhs_str_p++; - if (c == '\0') { + if (c == NUL) { /* ';' separates cookies in a Cookie header, so it ends a value too. */ - if (u != '\0' && u != ' ' && u != '\t' && u != ':' && u != '?' && u != '=' && u != '\n' && u != '\r' && u != ';') + if (u != NUL && u != ' ' && u != '\t' && u != ':' && u != '?' && u != '=' && u != '\n' && u != '\r' && u != ';') return false; return true; } @@ -487,10 +487,10 @@ void handle_post(void) // Find end of request path while (*p && !is_separator(*p)) p++; - *p++ = '\0'; + *p++ = NUL; // Find end of request header - boundary[0] ='\0'; + boundary[0] =NUL; p = scan_header(p); dbg_string("Boundary: >"); dbg_string_x(boundary); dbg_string("<\n"); if (!*p || !content_type) { @@ -550,7 +550,7 @@ void handle_post(void) dbg_string("Password accepted!\n"); read_reg_timer(&last_session_use); gen_random_bytes(session_id, SESSION_ID_LENGTH); - session_id[SESSION_ID_LENGTH] = '\0'; + session_id[SESSION_ID_LENGTH] = NUL; slen = strtox(outbuf, "HTTP/1.1 302 Found\r\nConnection: close\r\nLocation: index.html\r\n" \ "Set-Cookie: session="); for (register uint8_t i = 0; i < SESSION_ID_LENGTH; i++) @@ -736,7 +736,7 @@ void httpd_appcall(void) __xdata uint8_t *q = p; while (*p && !is_separator(*p)) p++; - *p = '\0'; + *p = NUL; dbg_string_x(q); dbg_char('\n'); diff --git a/rtl837x_common.h b/rtl837x_common.h index 27d9449..8f7bf1b 100644 --- a/rtl837x_common.h +++ b/rtl837x_common.h @@ -8,6 +8,7 @@ #define SYS_TICK_HZ 200 #define CPU_PORT 9 +#define NUL '\0' // Define Port-masks for 9-port devices and 6-port devices #define PMASK_9 0x1ff diff --git a/rtlplayground.c b/rtlplayground.c index d94d0fb..a7f1dc5 100644 --- a/rtlplayground.c +++ b/rtlplayground.c @@ -1257,11 +1257,11 @@ bool sfp_read_field(__xdata char *dst, uint8_t sfp, uint8_t start, uint8_t lengt if (!sfp_read_block(sfp, start, length)) return false; - dst[length] = '\0'; + dst[length] = NUL; memcpy(dst, sfp_buf, length); while (length > 0 && dst[--length] == ' ') - dst[length] = '\0'; + dst[length] = NUL; return true; } @@ -2069,7 +2069,7 @@ void check_and_flash_update_image(void) * because itohex() is inline and brings its own frame. */ void set_hostname_default(void) { - if (hostname[0] != '\0') + if (hostname[0] != NUL) return; strcpy((__xdata uint8_t *)hostname, "RTLPlayground-"); @@ -2079,7 +2079,7 @@ void set_hostname_default(void) hostname[17] = hex[uip_ethaddr.addr[4] & 0xf]; hostname[18] = hex[uip_ethaddr.addr[5] >> 4]; hostname[19] = hex[uip_ethaddr.addr[5] & 0xf]; - hostname[20] = '\0'; + hostname[20] = NUL; } From e62e0fc8bd45969528d369086da8e45556d5a7c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Tue, 18 Aug 2026 23:06:12 +0200 Subject: [PATCH 19/26] CMD: Isolate allow the CPU-port as a member. --- cmd_parser.c | 30 +++++++++++++++++++++++++++++- rtlplayground.c | 4 ++-- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index d75233f..2d64708 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -269,6 +269,34 @@ uint8_t cmd_parse_port_separator(uint8_t idx) { return ret; } +// Same as cmd_parse_port_separator() addition to allow the CPU-port. +// returns 0 when on parser error or invalid value or no space or no NUL. +// return non-zero number of bytes consumed including the space. +uint8_t cmd_parse_port_cpu_separator(uint8_t idx) { + uint8_t ret = atoi_byte(idx); + // Check valid con + if (ret != 0) { + // port number 1-10 -> 0-9 + atoi_results_u8 -= 1; + if(atoi_results_u8 < CPU_PORT) + // Phy port, translate it. + ret = cmd_parse_port(idx); + // Check is not + else if (atoi_results_u8 > CPU_PORT) + ret = 0; + } + + if (ret != 0) { + idx += ret; + uint8_t c = cmd_buffer[idx]; + if (c == ' ') { + ret++; + } else if (c != NUL) + ret = 0; + } + return ret; +} + // check if the cmd_buffer[idx] is a space. __bit cmd_is_space(uint8_t idx) { @@ -562,7 +590,7 @@ void parse_isolate(void) uint8_t w = 2; while (w < cmd_words_len) { - if (cmd_parse_port_separator(cmd_words_b[w++]) == 0) + if (cmd_parse_port_cpu_separator(cmd_words_b[w++]) == 0) goto err; uint8_t port = atoi_results_u8; members |= ((uint16_t)1) << port; diff --git a/rtlplayground.c b/rtlplayground.c index a7f1dc5..63a9c39 100644 --- a/rtlplayground.c +++ b/rtlplayground.c @@ -816,9 +816,9 @@ void print_reg(uint16_t reg) // Print the phy port for a log port number. void print_port(uint8_t port) { - if (port < 9) + if (port < CPU_PORT) write_char(machine.log_to_phys_port[port] + '0'); - else if (port == 9) + else if (port == CPU_PORT) print_string("CPU"); else { print_string("UNKNOWN "); From 86532160479d1e9d3c1700ba01a673d6992cace4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Wed, 19 Aug 2026 19:23:51 +0200 Subject: [PATCH 20/26] CMD: Fix some comments --- cmd_parser.c | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index 2d64708..3ef2fe3 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -113,7 +113,7 @@ uint8_t cmd_compare(uint8_t start, __code uint8_t * cmd) uint8_t c = cmd[j]; uint8_t b = cmd_buffer[i]; - // cmd is garanteerd to be NULL-terminated. + // cmd is garanteerd to be NUL-terminated. if (c == NUL) { if ((b == ' ') || (b == NUL)) { // Match @@ -253,9 +253,9 @@ uint8_t cmd_parse_port(uint8_t idx) { } -// Same as cmd_parse_port() addition to check the trailing space or NUL. -// returns 0 when on parser error or invalid value or no space or no NUL. -// return non-zero number of bytes consumed including the space. +// Same as cmd_parse_port() addition to check the trailing SPACE or NUL. +// returns 0 when on parser error or invalid value or no SPACE or no NUL. +// return non-zero number of bytes consumed including the SPACE. uint8_t cmd_parse_port_separator(uint8_t idx) { uint8_t ret = cmd_parse_port(idx); if (ret != 0) { @@ -270,18 +270,17 @@ uint8_t cmd_parse_port_separator(uint8_t idx) { } // Same as cmd_parse_port_separator() addition to allow the CPU-port. -// returns 0 when on parser error or invalid value or no space or no NUL. -// return non-zero number of bytes consumed including the space. +// returns 0 when on parser error or invalid value or no SPACE or no NUL. +// return non-zero number of bytes consumed including the SPACE. uint8_t cmd_parse_port_cpu_separator(uint8_t idx) { uint8_t ret = atoi_byte(idx); - // Check valid con + // Check valid conversion if (ret != 0) { // port number 1-10 -> 0-9 atoi_results_u8 -= 1; if(atoi_results_u8 < CPU_PORT) // Phy port, translate it. ret = cmd_parse_port(idx); - // Check is not else if (atoi_results_u8 > CPU_PORT) ret = 0; } @@ -298,20 +297,20 @@ uint8_t cmd_parse_port_cpu_separator(uint8_t idx) { } -// check if the cmd_buffer[idx] is a space. +// check if the cmd_buffer[idx] is a SPACE. __bit cmd_is_space(uint8_t idx) { return cmd_buffer[idx] == ' '; } -// check if the cmd_buffer[idx] is a space or NULL. +// check if the cmd_buffer[idx] is a SPACE or NUL. __bit cmd_is_space_or_nul(uint8_t idx) { uint8_t c = cmd_buffer[idx]; return c == ' ' || c == NUL; } -// returns 0 when on parser error or invalid value or no space. -// return non-zero number of bytes consumed including the space. -// Stops at a space or NUL. +// returns 0 when on parser error or invalid value or no SPACE. +// return non-zero number of bytes consumed including the SPACE. +// Stops at a SPACE or NUL. uint8_t parse_ip(uint8_t idx) { uint8_t b = 0; @@ -1268,7 +1267,7 @@ void parse_rnd(void) void parse_passwd(void) { - // cmd_words_len can be more then 2 if a space in the password. + // cmd_words_len can be more then 2 if a SPACE in the password. if (cmd_words_len >= 2) { uint8_t i = cmd_words_b[1]; uint8_t c = 0; @@ -1776,7 +1775,7 @@ void cmd_parser(void) __banked if (save_cmd && cmd_words_len) { - // Find end of the cmd-buffer, looking for the NULL-byte. + // Find end of the cmd-buffer, looking for the NUL-byte. uint8_t i = cmd_words_b[cmd_words_len - 1]; do { i++; From f511453f79997e4e247cd3577661de40c106dcdd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Wed, 19 Aug 2026 19:31:21 +0200 Subject: [PATCH 21/26] CMD: refactor code --- cmd_parser.c | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index 3ef2fe3..d0ba703 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -368,7 +368,6 @@ void parse_lag(void) { __xdata uint8_t group; __xdata uint16_t members = 0; - uint8_t ret; if (cmd_compare(1, "show")) { print_string("LAG status:\n"); @@ -398,9 +397,8 @@ void parse_lag(void) if (cmd_words_len < 2) goto err; - // Parse group, expect only one number 0-9. - ret = atoi_byte(cmd_words_b[1]); - if (ret != 1) + // Parse group, expect only one number 0-9; + if (atoi_byte(cmd_words_b[1]) != 1) goto err; group = atoi_results_u8 - 1; @@ -834,7 +832,7 @@ void parse_port(void) void parse_mtu(void) { - uint8_t p, ret; + uint8_t p; if (cmd_compare(1, "show")) { for (p = machine.min_port; p <= machine.max_port; p++) { @@ -854,9 +852,7 @@ void parse_mtu(void) p = atoi_results_u8; print_byte(p); - ret = atoi_short(cmd_words_b[2]); - - if (!ret || atoi_results_short < 64 || atoi_results_short > 0x3fff) { + if (atoi_short(cmd_words_b[2]) == 0 || atoi_results_short < 64 || atoi_results_short > 0x3fff) { print_string("MTU must be 64..16383\n"); return; } @@ -959,8 +955,6 @@ err: void parse_regget(void) { - uint16_t reg = 0; - if (cmd_words_len != 2) { goto err; } @@ -971,7 +965,7 @@ void parse_regget(void) goto err; } - reg = hexvalue[0]; + uint16_t reg = hexvalue[0]; if (hex_size == 2) { reg <<= 8; reg |= hexvalue[1]; @@ -994,8 +988,6 @@ err: void parse_regset(void) { - uint16_t reg = 0; - if (cmd_words_len != 3) { goto err; } @@ -1005,7 +997,7 @@ void parse_regset(void) goto err; } - reg = hexvalue[0]; + uint16_t reg = hexvalue[0]; if (hex_size == 2) { reg <<= 8; reg |= hexvalue[1]; From 95b3bde82276bd081fcf6b3296acfff1f6d695fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Wed, 19 Aug 2026 19:34:34 +0200 Subject: [PATCH 22/26] Rename print_port() to print_phys_port() --- cmd_parser.c | 16 ++++++++-------- rtl837x_common.h | 2 +- rtl837x_phy.c | 4 ++-- rtl837x_port.c | 8 ++++---- rtlplayground.c | 4 ++-- 5 files changed, 17 insertions(+), 17 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index d0ba703..848a74f 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -381,7 +381,7 @@ void parse_lag(void) print_string(" member ports: "); for (uint8_t j = 0; j < 10; j++) { if (members & 1) { - print_port(j); + print_phys_port(j); write_char(' '); } members >>= 1; @@ -569,7 +569,7 @@ void parse_isolate(void) members = port_isolation_get(port_configured); for (uint8_t i = 0; i < 10; i++) { if (members & 1) { - print_port(i); + print_phys_port(i); write_char(' '); } members >>= 1; @@ -634,7 +634,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)) { - print_string("Error setting ingress filter for port "); print_port(log_port); write_char('\n'); + print_string("Error setting ingress filter for port "); print_phys_port(log_port); write_char('\n'); return; } print_string("All ports ingress filter set to: "); @@ -652,14 +652,14 @@ void parse_ingress(void) idx += ret; if (!vlan_ingress_mode_parse(cmd_buffer[idx++], &mode) || !cmd_is_space_or_nul(idx)) { - print_string("Invalid ingress mode for port "); print_port(log_port); print_string(" in ingress command\n"); + 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)) { - print_string("Error setting ingress filter for port "); print_port(log_port); write_char('\n'); + print_string("Error setting ingress filter for port "); print_phys_port(log_port); write_char('\n'); return; } - print_string("Port "); print_port(log_port); + print_string("Port "); print_phys_port(log_port); print_string(" ingress filter set to: "); print_port_ingress_filter_mode(mode); write_char('\n'); } @@ -684,7 +684,7 @@ void parse_mirror(void) print_string("NOT Enabled: "); } print_string("Mirroring port: "); - print_port(mirroring_port); + print_phys_port(mirroring_port); reg_read_m(RTL837x_MIRROR_CONF); uint16_t m = sfr_data[0]; m = (m << 8) | sfr_data[1]; @@ -838,7 +838,7 @@ void parse_mtu(void) for (p = machine.min_port; p <= machine.max_port; p++) { reg_read_m(RTL8373_REG_MAC_L2_PORT_MAX_LEN + ((uint16_t) p << 8)); uint16_t mtu = SFR_DATA_U16 & 0x3fff; - print_string("Port "); print_port(p); + print_string("Port "); print_phys_port(p); write_char(' '); print_short(mtu); write_char('\n'); } return; diff --git a/rtl837x_common.h b/rtl837x_common.h index 8f7bf1b..432260e 100644 --- a/rtl837x_common.h +++ b/rtl837x_common.h @@ -136,7 +136,7 @@ void itoa(uint8_t v); void print_sfr_data(void); void print_phy_data(void); void print_cmd_prompt(void); -void print_port(uint8_t port); +void print_phys_port(uint8_t port); void phy_write_mask(uint16_t phy_mask, uint8_t dev_id, uint16_t reg, uint16_t v); void phy_write(uint8_t phy_id, uint8_t dev_id, uint16_t reg, uint16_t v); void phy_read(uint8_t phy_id, uint8_t dev_id, uint16_t reg); diff --git a/rtl837x_phy.c b/rtl837x_phy.c index 5512755..240d1d0 100644 --- a/rtl837x_phy.c +++ b/rtl837x_phy.c @@ -262,7 +262,7 @@ void phy_set_speed(void) __banked { uint16_t v; - print_string("Setting port "); print_port(phy_settings.port); + print_string("Setting port "); print_phys_port(phy_settings.port); if (machine.n_10g && phy_settings.port == 3) phy_settings.is10g_port = 1; if (machine.n_10g == 2 && phy_settings.port == 8) @@ -381,7 +381,7 @@ void phy_set_duplex(void) __banked { uint16_t v; - print_string("Setting port "); print_port(phy_settings.port); + print_string("Setting port "); print_phys_port(phy_settings.port); if (phy_settings.duplex) print_string(" to full duplex"); else diff --git a/rtl837x_port.c b/rtl837x_port.c index bc073d0..103ac4e 100644 --- a/rtl837x_port.c +++ b/rtl837x_port.c @@ -390,7 +390,7 @@ void port_l2_learned(void) __banked print_string("\tlearned\t"); port |= (sfr_data[3] & 0x3) << 2; - print_port(port); + print_phys_port(port); } entry++; @@ -428,7 +428,7 @@ void port_stats_print(void) __banked { print_string("\nPort\tState\tLink\tTxGood\t\tTxBad\t\tRxGood\t\tRxBad\n"); for (uint8_t i = machine.min_port; i <= machine.max_port; i++) { - print_port(i); write_char('\t'); + print_phys_port(i); write_char('\t'); if (!machine.is_sfp[i]) { phy_read(i, PHY_MMD31, 0xa610); @@ -603,7 +603,7 @@ void port_eee_disable(uint8_t port) __banked void port_eee_status(uint8_t port) __banked { - print_string("Port: "); print_port(port); + print_string("Port: "); print_phys_port(port); print_string(": "); if (machine.is_sfp[port]) { print_string("SFP\n"); @@ -800,7 +800,7 @@ void print_port_ingress_filter_mode(vlan_ingress_mode_t mode) __banked void print_vlan_ingress_port(uint8_t log_port) __banked { - print_port(log_port);write_char('\t'); + print_phys_port(log_port);write_char('\t'); print_short(port_pvid_get(log_port));write_char('\t'); print_port_ingress_filter_mode(port_ingress_filter_get(log_port));write_char('\t'); port_ingress_vlan_filter_get(log_port) ? print_string("Enabled") : print_string("Disabled"); diff --git a/rtlplayground.c b/rtlplayground.c index 63a9c39..f0a3292 100644 --- a/rtlplayground.c +++ b/rtlplayground.c @@ -813,8 +813,8 @@ void print_reg(uint16_t reg) print_sfr_data(); } -// Print the phy port for a log port number. -void print_port(uint8_t port) +// Print the physical port of a logical port number. +void print_phys_port(uint8_t port) { if (port < CPU_PORT) write_char(machine.log_to_phys_port[port] + '0'); From 1212def7992db3496add2679dc172b724a34f5f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Thu, 20 Aug 2026 19:37:34 +0200 Subject: [PATCH 23/26] CMD: fixes many comments --- cmd_parser.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index 848a74f..7273582 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -113,7 +113,7 @@ uint8_t cmd_compare(uint8_t start, __code uint8_t * cmd) uint8_t c = cmd[j]; uint8_t b = cmd_buffer[i]; - // cmd is garanteerd to be NUL-terminated. + // cmd is guaranteed to be NUL-terminated. if (c == NUL) { if ((b == ' ') || (b == NUL)) { // Match @@ -187,8 +187,8 @@ uint8_t atoi_hex(uint8_t idx) } -// return 0 on error or non-zero number of number-byte taken for the conversion. -// Stops at any non-digit '0'-'9' char or bytes is more then 3. +// Returns 0 or the number of digits taken into account for conversion. +// Stops at any non-digit '0'-'9' char or more than 3 bytes. uint8_t atoi_byte(uint8_t idx) { uint8_t cnt = 0; @@ -211,7 +211,7 @@ uint8_t atoi_byte(uint8_t idx) } -// return 0 on error or non-zero number of number-byte taken for the conversion. +// Returns 0 or the number of digits taken into account for conversion. // Stops at any non-digit '0'-'9' char or bytes is more then 5. uint8_t atoi_short(uint8_t idx) { @@ -234,9 +234,9 @@ uint8_t atoi_short(uint8_t idx) } /* Parse, validate and translate phys_to_log_port physical port argument. - * So CPU-port 10 is not valid. + * The CPU-port, i.e. port 10, is not a valid argument. * returns 0 when on parser error or invalid value. - * return non-zero number of bytes consumed. + * returns non-zero number of characters consumed. * Store the value in atoi_results_u8. */ uint8_t cmd_parse_port(uint8_t idx) { @@ -253,9 +253,9 @@ uint8_t cmd_parse_port(uint8_t idx) { } -// Same as cmd_parse_port() addition to check the trailing SPACE or NUL. +// Same as cmd_parse_port() but additionally check for trailing SPACE or NUL. // returns 0 when on parser error or invalid value or no SPACE or no NUL. -// return non-zero number of bytes consumed including the SPACE. +// returns non-zero number of characters consumed including the SPACE. uint8_t cmd_parse_port_separator(uint8_t idx) { uint8_t ret = cmd_parse_port(idx); if (ret != 0) { @@ -269,9 +269,9 @@ uint8_t cmd_parse_port_separator(uint8_t idx) { return ret; } -// Same as cmd_parse_port_separator() addition to allow the CPU-port. +// Same as cmd_parse_port_separator() but additionally allow CPU-port. // returns 0 when on parser error or invalid value or no SPACE or no NUL. -// return non-zero number of bytes consumed including the SPACE. +// returns number of characters consumed including the SPACE. uint8_t cmd_parse_port_cpu_separator(uint8_t idx) { uint8_t ret = atoi_byte(idx); // Check valid conversion @@ -308,9 +308,9 @@ __bit cmd_is_space_or_nul(uint8_t idx) { return c == ' ' || c == NUL; } -// returns 0 when on parser error or invalid value or no SPACE. -// return non-zero number of bytes consumed including the SPACE. -// Stops at a SPACE or NUL. +// Parse an IPv4 address +// returns 0 when on parse error or invalid value or it don't ends with SPACE or NUL. +// returns non-zero number of characters consumed including the SPACE. uint8_t parse_ip(uint8_t idx) { uint8_t b = 0; @@ -347,7 +347,7 @@ err: return 0; } -// Print a IPv4 adress. +// Prints an IPv4 address. void print_ip(__xdata uint8_t * ptr) { uint8_t idx = 0; From beb14deba56dbbda1e1b93ba07514c6ffc120d5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Sun, 23 Aug 2026 14:46:02 +0200 Subject: [PATCH 24/26] httpd: send_counter(): Validate phys_port_idx and better error handling --- httpd/httpd.c | 9 +-------- httpd/page_impl.c | 30 ++++++++++++++++++++++-------- httpd/page_impl.h | 4 +++- 3 files changed, 26 insertions(+), 17 deletions(-) diff --git a/httpd/httpd.c b/httpd/httpd.c index 8f52631..411ba0c 100644 --- a/httpd/httpd.c +++ b/httpd/httpd.c @@ -758,16 +758,9 @@ void httpd_appcall(void) parse_short(q + 15); send_vlan(short_parsed); } else if (is_word(q, "/counters.json")) { - /* The port is one raw character of the request line and - * indexes a nine entry table, so bound it here instead - * of trusting the client to have sent a digit. Anything - * below '0' wraps well past eight, so the one test - * covers both ends. */ uint8_t cport = q[20] - '0'; - if (cport > 8) + if (send_counters(cport)) send_bad_request(); - else - send_counters(cport); } else if (is_word(q, "/eee.json")) { send_eee(); } else if (is_word(q, "/bandwidth.json")) { diff --git a/httpd/page_impl.c b/httpd/page_impl.c index 92aeb1a..4bcf497 100644 --- a/httpd/page_impl.c +++ b/httpd/page_impl.c @@ -289,17 +289,25 @@ void send_vlan(uint16_t vlan) slen += strtox(outbuf + slen, "\"}"); } - -void send_counters(char port) +/* Send counters + * Only accepts physical port idx to 0-8. + * Returns an error if the port physical don't exists. + */ +bool send_counters(uint8_t phys_port_idx) { - dbg_string("send_counters called: "); dbg_byte(port); dbg_char('\n'); + if (phys_port_idx > 8) + goto err; + uint8_t log_port = machine.phys_to_log_port[phys_port_idx]; + if (log_port == 0) + goto err; + + dbg_string("send_counters called: "); dbg_byte(phys_port_idx); dbg_char('\n'); slen = strtox(outbuf, HTTP_RESPONCE_JSON); - dbg_string("sending counters\n"); - dbg_byte(port); - uint8_t i = machine.phys_to_log_port[port]; - slen += strtox(outbuf + slen, "["); + dbg_string("sending counters\n"); dbg_byte(phys_port_idx); + + char_to_html('['); for (uint8_t counter = 0; counter < 0x37; counter++) { - STAT_GET(counter, i); + STAT_GET(counter, log_port); slen += strtox(outbuf + slen, "\"0x"); reg_to_html(RTL837X_STAT_V_HIGH); reg_to_html_long(RTL837X_STAT_V_LOW); @@ -308,6 +316,12 @@ void send_counters(char port) char_to_html(','); } char_to_html(']'); + + return false; + +err: + dbg_string("Error: counters: phy_port_idx don't exists\n"); + return true; } diff --git a/httpd/page_impl.h b/httpd/page_impl.h index 7907285..1cbd94e 100644 --- a/httpd/page_impl.h +++ b/httpd/page_impl.h @@ -1,7 +1,9 @@ #ifndef __PAGE_IMPL_H__ #define __PAGE_IMPL_H__ -void send_counters(char port); +#include + +bool send_counters(uint8_t phys_port_idx); void send_status(void); void send_vlan(uint16_t vlan); void send_basic_info(void); From 74013bb14fec8754c991671b5a2845f335d1de70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Sun, 23 Aug 2026 15:33:04 +0200 Subject: [PATCH 25/26] CMD: Treat port zero as CPU-port --- cmd_parser.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index 7273582..66b05bd 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -234,7 +234,7 @@ uint8_t atoi_short(uint8_t idx) } /* Parse, validate and translate phys_to_log_port physical port argument. - * The CPU-port, i.e. port 10, is not a valid argument. + * The CPU-port, i.e. port 0, is not a valid argument. * returns 0 when on parser error or invalid value. * returns non-zero number of characters consumed. * Store the value in atoi_results_u8. @@ -273,16 +273,11 @@ uint8_t cmd_parse_port_separator(uint8_t idx) { // returns 0 when on parser error or invalid value or no SPACE or no NUL. // returns number of characters consumed including the SPACE. uint8_t cmd_parse_port_cpu_separator(uint8_t idx) { - uint8_t ret = atoi_byte(idx); - // Check valid conversion - if (ret != 0) { - // port number 1-10 -> 0-9 - atoi_results_u8 -= 1; - if(atoi_results_u8 < CPU_PORT) - // Phy port, translate it. - ret = cmd_parse_port(idx); - else if (atoi_results_u8 > CPU_PORT) - ret = 0; + uint8_t ret = cmd_parse_port(idx); + + if (ret == 0 && cmd_buffer[idx] == '0') { + ret = 1; + atoi_results_u8 = CPU_PORT; } if (ret != 0) { From ee05cd400088b8adc42e4b0bccee98543bd4f1a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Mon, 24 Aug 2026 22:03:50 +0200 Subject: [PATCH 26/26] Fix: `send_counters()`, `port`-argument have to be physical port number. Not the physical port index! --- html/stat.js | 42 +++++++++++++++++++++--------------------- httpd/page_impl.c | 5 +++-- httpd/page_impl.h | 2 +- 3 files changed, 25 insertions(+), 24 deletions(-) diff --git a/html/stat.js b/html/stat.js index 2709eeb..0944d96 100644 --- a/html/stat.js +++ b/html/stat.js @@ -109,7 +109,7 @@ const mib_counters = [ function getCounters(port) { var xhttp = new XMLHttpRequest(); const popup = document.getElementById('popup'); - xhttp.onreadystatechange = function() { + xhttp.onreadystatechange = function () { if (this.readyState == 4 && this.status == 200) { const s = JSON.parse(xhttp.responseText); console.log("Counters: ", JSON.stringify(s)); @@ -118,16 +118,16 @@ function getCounters(port) { console.log("Counter 0: ", BigInt(s[0]).toString(), " length: ", s.length); var c = 0; for (i = 0; i < mib_counters.length; i += 4) { - console.log(i, " ", mib_counters[i], ": ", mib_counters[i+1]); + console.log(i, " ", mib_counters[i], ": ", mib_counters[i + 1]); if (mib_counters[i] == "" && mib_counters[i + 1] == 8) { console.log("c " + i + ": continue"); continue; } - var count = BigInt(s[i/4]); - if (mib_counters[i+1] == 8) { + var count = BigInt(s[i / 4]); + if (mib_counters[i + 1] == 8) { tableHtml += "" + mib_counters[i] + "" + count.toString() + ""; c += 1; - } else if (mib_counters[i+1] == 4) { + } else if (mib_counters[i + 1] == 4) { if (mib_counters[i] != "") { tableHtml += "" + mib_counters[i] + "" + (count >> 32n).toString() + ""; c += 1; @@ -136,8 +136,8 @@ function getCounters(port) { tableHtml += " "; c = 0; } - if (mib_counters[i+2] != "") { - tableHtml += "" + mib_counters[i+2] + "" + (count & 4294967295n).toString() + ""; + if (mib_counters[i + 2] != "") { + tableHtml += "" + mib_counters[i + 2] + "" + (count & 4294967295n).toString() + ""; c += 1; } } @@ -161,26 +161,26 @@ function fillStats() { return; if (tbl.rows.length > 1) { for (let i = 0; i < numPorts; i++) { - console.log("Table Update row: " + i + " state " + pState[i] + " is " + linkS[pState[i] +1]); - tbl.rows[i+1].cells[2].innerHTML = linkText(pState[i]+1); - tbl.rows[i+1].cells[3].innerHTML = `${txG[i]}` + t('common_pkts'); - tbl.rows[i+1].cells[4].innerHTML = `${txB[i]}` + t('common_pkts'); - tbl.rows[i+1].cells[5].innerHTML = `${rxG[i]}` + t('common_pkts'); - tbl.rows[i+1].cells[6].innerHTML = `${rxB[i]}` + t('common_pkts'); + console.log("Table Update row: " + i + " state " + pState[i] + " is " + linkS[pState[i] + 1]); + tbl.rows[i + 1].cells[2].innerHTML = linkText(pState[i] + 1); + tbl.rows[i + 1].cells[3].innerHTML = `${txG[i]}` + t('common_pkts'); + tbl.rows[i + 1].cells[4].innerHTML = `${txB[i]}` + t('common_pkts'); + tbl.rows[i + 1].cells[5].innerHTML = `${rxG[i]}` + t('common_pkts'); + tbl.rows[i + 1].cells[6].innerHTML = `${rxB[i]}` + t('common_pkts'); } } else { for (let i = 0; i < numPorts; i++) { console.log("Table row: " + i); const tr = tbl.insertRow(); - let td = tr.insertCell(); td.appendChild(document.createTextNode(t('common_port') + (i+1))); + let td = tr.insertCell(); td.appendChild(document.createTextNode(t('common_port') + (i + 1))); let portName = portNames[physToLogPort[i]] || ''; td = tr.insertCell(); td.appendChild(document.createTextNode(portName)); - td = tr.insertCell(); td.appendChild(document.createTextNode(linkText(pState[i]+1))); + td = tr.insertCell(); td.appendChild(document.createTextNode(linkText(pState[i] + 1))); td = tr.insertCell(); td.appendChild(document.createTextNode(`${txG[i]}` + t('common_pkts'))); - td = tr.insertCell();td.appendChild(document.createTextNode(`${txB[i]}` + t('common_pkts'))); - td = tr.insertCell();td.appendChild(document.createTextNode(`${rxG[i]}` + t('common_pkts'))); - td = tr.insertCell();td.appendChild(document.createTextNode(`${rxB[i]}` + t('common_pkts'))); - var button = ''; + td = tr.insertCell(); td.appendChild(document.createTextNode(`${txB[i]}` + t('common_pkts'))); + td = tr.insertCell(); td.appendChild(document.createTextNode(`${rxG[i]}` + t('common_pkts'))); + td = tr.insertCell(); td.appendChild(document.createTextNode(`${rxB[i]}` + t('common_pkts'))); + var button = ''; td = tr.insertCell(); td.innerHTML = button; } } @@ -197,8 +197,8 @@ window.addEventListener('click', (event) => { } }); -window.addEventListener("load", function() { - update( () => { +window.addEventListener("load", function () { + update(() => { update(); fillStats(); const stat = setInterval(fillStats, 1000); diff --git a/httpd/page_impl.c b/httpd/page_impl.c index 4bcf497..d55d4fe 100644 --- a/httpd/page_impl.c +++ b/httpd/page_impl.c @@ -290,11 +290,12 @@ void send_vlan(uint16_t vlan) } /* Send counters - * Only accepts physical port idx to 0-8. + * Only accepts physical port 1..9. * Returns an error if the port physical don't exists. */ -bool send_counters(uint8_t phys_port_idx) +bool send_counters(uint8_t phys_port) { + uint8_t phys_port_idx = phys_port - 1; if (phys_port_idx > 8) goto err; uint8_t log_port = machine.phys_to_log_port[phys_port_idx]; diff --git a/httpd/page_impl.h b/httpd/page_impl.h index 1cbd94e..02ed4eb 100644 --- a/httpd/page_impl.h +++ b/httpd/page_impl.h @@ -3,7 +3,7 @@ #include -bool send_counters(uint8_t phys_port_idx); +bool send_counters(uint8_t phys_port); void send_status(void); void send_vlan(uint16_t vlan); void send_basic_info(void);