This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
Neels Hofmeyr gerrit-no-reply at lists.osmocom.orgHello Harald Welte, Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/5437 to look at the new patch set (#4). ctrl: on parse errors, return a detailed message to sender The recently added ctrl_cmd_parse2() returns non-NULL cmd with error messages upon parsing errors. In handle_control_read(), use ctrl_cmd_parse2() and send those back to the CTRL command sender as reply. Retain the previous "Command parser error" reply only in case ctrl_cmd_parse2() should return NULL, which shouldn't actually happen at all. Change-Id: Ie35a02555b76913bb12734a76fc40fde7ffb244d --- M src/ctrl/control_if.c M tests/ctrl/ctrl_test.c 2 files changed, 25 insertions(+), 13 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/37/5437/4 diff --git a/src/ctrl/control_if.c b/src/ctrl/control_if.c index 5c73b63..17a012a 100644 --- a/src/ctrl/control_if.c +++ b/src/ctrl/control_if.c @@ -381,14 +381,10 @@ msg->l2h = iph_ext->data; - cmd = ctrl_cmd_parse(ccon, msg); + cmd = ctrl_cmd_parse2(ccon, msg); - if (cmd) { - cmd->ccon = ccon; - if (ctrl_cmd_handle(ctrl, cmd, ctrl->data) != CTRL_CMD_HANDLED) { - ctrl_cmd_send(&ccon->write_queue, cmd); - } - } else { + if (!cmd) { + /* should never happen */ cmd = talloc_zero(ccon, struct ctrl_cmd); if (!cmd) return -ENOMEM; @@ -396,10 +392,23 @@ cmd->type = CTRL_TYPE_ERROR; cmd->id = "err"; cmd->reply = "Command parser error."; - ctrl_cmd_send(&ccon->write_queue, cmd); } - talloc_free(cmd); + if (cmd->type != CTRL_TYPE_ERROR) { + cmd->ccon = ccon; + if (ctrl_cmd_handle(ctrl, cmd, ctrl->data) == CTRL_CMD_HANDLED) { + /* On CTRL_CMD_HANDLED, no reply needs to be sent back. */ + talloc_free(cmd); + cmd = NULL; + } + } + + if (cmd) { + /* There is a reply or error that should be reported back to the sender. */ + ctrl_cmd_send(&ccon->write_queue, cmd); + talloc_free(cmd); + } + return 0; } @@ -894,13 +903,16 @@ osmo_strlcpy((char *)msg->data, cmdstr, msgb_tailroom(msg)); msgb_put(msg, strlen(cmdstr)); - cmd = ctrl_cmd_parse(ch, msg); + cmd = ctrl_cmd_parse2(ch, msg); msgb_free(msg); if (!cmd) return NULL; - if (ctrl_cmd_handle(ch, cmd, NULL) < 0) { + if (cmd->type == CTRL_TYPE_ERROR) + return cmd; + if (ctrl_cmd_handle(ch, cmd, NULL) == CTRL_CMD_HANDLED) { + /* No reply should be sent back. */ talloc_free(cmd); - return NULL; + cmd = NULL; } return cmd; } diff --git a/tests/ctrl/ctrl_test.c b/tests/ctrl/ctrl_test.c index b1d4f23..39ec61a 100644 --- a/tests/ctrl/ctrl_test.c +++ b/tests/ctrl/ctrl_test.c @@ -76,7 +76,7 @@ printf("test: '%s'\n", osmo_escape_str(t->cmd_str, -1)); printf("parsing:\n"); - cmd = ctrl_cmd_parse(ctx, msg); + cmd = ctrl_cmd_parse2(ctx, msg); OSMO_ASSERT(cmd); OSMO_ASSERT(t->expect_parsed.type == cmd->type); -- To view, visit https://gerrit.osmocom.org/5437 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie35a02555b76913bb12734a76fc40fde7ffb244d Gerrit-PatchSet: 4 Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Owner: Neels Hofmeyr <nhofmeyr at sysmocom.de> Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org> Gerrit-Reviewer: Jenkins Builder