[PATCH] port-serial: remove response buffer when an error is returned

[Aleksander Morgado]

When valid responses were returned to the caller of the serial command, the
caller itself was responsible for removing from the GByteArray the data that
it was successfully processed (all the data in AT, just 1 message in QCDM). But,
the same logic was missing for the case of errors; we were not explicitly
removing the response and therefore in some cases we would see it propagated
into the next command response. It was common to see this issue when the echo
removal was disabled in the serial port, as in Option/HSO modems:

    <debug> (ttyHS3): --> 'AT+CNUM<CR>'
    <debug> (ttyHS3): <-- '<CR><LF>+CME ERROR: 14<CR><LF>'
    <debug> Got failure code 14: SIM busy
    <debug> (ttyHS3) device open count is 1 (close)
    <warn>  couldn't load list of Own Numbers: 'Failed to parse NV MDN command result: -17'
    <debug> (ttyHS3) device open count is 2 (open)
    <debug> (ttyHS3): --> 'AT_OPSYS?<CR>'
    <debug> (ttyHS3): <-- '<CR><LF>_OPSYS: 1,2<CR><LF><CR><LF>OK<CR><LF>'
    <warn>  couldn't load current allowed/preferred modes: 'Couldn't parse OPSYS response: '+CME ERROR: 14
    _OPSYS: 1,2''
---
 src/mm-port-serial.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/src/mm-port-serial.c b/src/mm-port-serial.c
index d1baa1b..53f8725 100644
--- a/src/mm-port-serial.c
+++ b/src/mm-port-serial.c
@@ -716,14 +716,26 @@ port_serial_got_response (MMPortSerial *self,
 
     ctx = (CommandContext *) g_queue_pop_head (self->priv->queue);
     if (ctx) {
-        if (error)
+        if (error) {
+            /* If we're returning an error parsed in a generic way from the inputs,
+             * we fully avoid returning a response bytearray. This really applies
+             * only to AT, not to QCDM, so we shouldn't be worried of losing chunks
+             * of the next QCDM message. And given that the caller won't get the
+             * response array, we're the ones in charge of removing the processed
+             * data (otherwise ERROR replies may get fed to the next response
+             * parser).
+             */
             g_simple_async_result_set_from_error (ctx->result, error);
-        else {
-            if (ctx->allow_cached && !error)
+            g_byte_array_remove_range (self->priv->response, 0, self->priv->response->len);
+        } else {
+            if (ctx->allow_cached)
                 port_serial_set_cached_reply (self, ctx->command, self->priv->response);
 
             /* Upon completion, it is a task of the caller to remove from the response
-             * buffer the processed data */
+             * buffer the processed data. This may seem unnecessary in the case of AT
+             * commands, as it'll remove the full received string, but the step is
+             * a key thing in the case of QCDM, where we want to read just until the
+             * next marker, not more. */
             g_simple_async_result_set_op_res_gpointer (ctx->result,
                                                        g_byte_array_ref (self->priv->response),
                                                        (GDestroyNotify)g_byte_array_unref);
-- 
2.7.0