From 67976f8686bbc93aa76e41cb8376a6a9cf3a7765 Mon Sep 17 00:00:00 2001 From: Marcin Niestroj Date: Thu, 19 Nov 2020 19:41:53 +0100 Subject: [PATCH] drivers: modem: verify send semaphore before taking any action Semaphore argument is checked only after requested command is already sent over modem interface. It makes little sense to return -EINVAL when half of the requested operation (send) has already been done. Check timeout and semaphore arguments just on the beginning of function. Return -EINVAL early, before sending any data or taking any locks. Clear out 'sem' variable when there is no need to wait. Use this variable later on as an indication to actually wait on semaphore or not. Signed-off-by: Marcin Niestroj --- drivers/modem/modem_cmd_handler.c | 34 +++++++++++++++---------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/drivers/modem/modem_cmd_handler.c b/drivers/modem/modem_cmd_handler.c index d318db0bf93..b191265212a 100644 --- a/drivers/modem/modem_cmd_handler.c +++ b/drivers/modem/modem_cmd_handler.c @@ -477,7 +477,15 @@ static int _modem_cmd_send(struct modem_iface *iface, struct modem_cmd_handler_data *data; int ret; - if (!iface || !handler || !handler->cmd_handler_data || !buf) { + if (!iface || !handler || !handler->cmd_handler_data || !buf || !sem) { + return -EINVAL; + } + + if (K_TIMEOUT_EQ(timeout, K_NO_WAIT)) { + /* semaphore is not needed if there is no timeout */ + sem = NULL; + } else if (!sem) { + /* cannot respect timeout without semaphore */ return -EINVAL; } @@ -509,23 +517,15 @@ static int _modem_cmd_send(struct modem_iface *iface, iface->write(iface, buf, strlen(buf)); iface->write(iface, data->eol, data->eol_len); - if (K_TIMEOUT_EQ(timeout, K_NO_WAIT)) { - ret = 0; - goto exit; - } + if (sem) { + k_sem_reset(sem); + ret = k_sem_take(sem, timeout); - if (!sem) { - ret = -EINVAL; - goto exit; - } - - k_sem_reset(sem); - ret = k_sem_take(sem, timeout); - - if (ret == 0) { - ret = data->last_error; - } else if (ret == -EAGAIN) { - ret = -ETIMEDOUT; + if (ret == 0) { + ret = data->last_error; + } else if (ret == -EAGAIN) { + ret = -ETIMEDOUT; + } } exit: