drivers: modem: modem_cmd_handler.c: Drop cmd from buf if args are missing
In 90c6dc5e7f45ba696de42ab2e02c0f31e07843e0, a change was introduced to
allow modem commands determine if they have enough data or not.
In a situation where some data is missing, the command should return
-EAGAIN and this should lead to another call of the command with more
data.
In this commit, the argument parser was also allowed to return -EAGAIN
to request more data due to missing arguments.
However, this can't work because in cmd_handler_process_rx_buf() before
calling process_cmd():
- we make sure that a CR/LF has been found.
- we compute match_len which can't be greater than the distance to the
next CR/LF.
Therefore, even if the command argument parser ask for more data by
returning -EAGAIN, next call will have the same value for match_len,
meaning that the parsing of argument will result in the same missing
argument situation.
This leads to an infinite loop of parsing the same data over and over in
an infinite loop.
This commit change this behavior to always drop the data in such a
situation. The command will not be answered and will therefore timeout,
but at least, next commands will correctly parse their returned data.
Signed-off-by: Xavier Chapron <xavier.chapron@stimio.fr>
diff --git a/drivers/modem/modem_cmd_handler.c b/drivers/modem/modem_cmd_handler.c
index 5b5bd31..04f6b76 100644
--- a/drivers/modem/modem_cmd_handler.c
+++ b/drivers/modem/modem_cmd_handler.c
@@ -159,7 +159,16 @@
/* missing arguments */
if (*argc < cmd->arg_count_min) {
- return -EAGAIN;
+ /* Do not return -EAGAIN here as there is no way new argument
+ * can be parsed later because match_len is computed to be
+ * the minimum of the distance to the first CRLF and the size
+ * of the buffer.
+ * Therefore, waiting more data on the interface won't change
+ * match_len value, which mean there is no point in waiting
+ * for more arguments, this will just end in a infinite loop
+ * parsing data and finding that some arguments are missing.
+ */
+ return -EINVAL;
}
/*
@@ -370,9 +379,13 @@
LOG_DBG("match cmd [%s] (len:%zu)",
log_strdup(cmd->cmd), match_len);
- if (process_cmd(cmd, match_len, data) == -EAGAIN) {
+ ret = process_cmd(cmd, match_len, data);
+ if (ret == -EAGAIN) {
k_sem_give(&data->sem_parse_lock);
break;
+ } else if (ret < 0) {
+ LOG_ERR("process cmd [%s] (len:%zu, ret:%d)",
+ log_strdup(cmd->cmd), match_len, ret);
}
/*
