<p>Pau Espin Pedrol <strong>merged</strong> this change.</p><p><a href="https://gerrit.osmocom.org/11765">View Change</a></p><div style="white-space:pre-wrap">Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified

</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">bts-trx: trx_if: Use struct to store CTRL msg parsed responses<br><br>Change-Id: Icb84bce0621042afa4301678ba1cc58d8e3662bb<br>---<br>M src/osmo-bts-trx/trx_if.c<br>1 file changed, 28 insertions(+), 22 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/osmo-bts-trx/trx_if.c b/src/osmo-bts-trx/trx_if.c</span><br><span>index f3de245..4bcdfc6 100644</span><br><span>--- a/src/osmo-bts-trx/trx_if.c</span><br><span>+++ b/src/osmo-bts-trx/trx_if.c</span><br><span>@@ -368,8 +368,13 @@</span><br><span>         return trx_ctrl_cmd(l1h, 1, "NOHANDOVER", "%d %d", tn, ss);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-static int parse_rsp(const char *buf_in, size_t len_in, char *cmdname_out, size_t cmdname_len,</span><br><span style="color: hsl(0, 100%, 40%);">-                  char *params_out, size_t params_len, int *status)</span><br><span style="color: hsl(120, 100%, 40%);">+struct trx_ctrl_rsp {</span><br><span style="color: hsl(120, 100%, 40%);">+      char cmd[50];</span><br><span style="color: hsl(120, 100%, 40%);">+ char params[100];</span><br><span style="color: hsl(120, 100%, 40%);">+     int status;</span><br><span style="color: hsl(120, 100%, 40%);">+};</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+static int parse_rsp(const char *buf_in, size_t len_in, struct trx_ctrl_rsp *rsp)</span><br><span> {</span><br><span>    char *p, *k;</span><br><span> </span><br><span>@@ -380,18 +385,18 @@</span><br><span>     if (!(p = strchr(buf_in + 4, ' ')))</span><br><span>          goto parse_err;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-     if (p - buf_in >= cmdname_len) {</span><br><span style="color: hsl(0, 100%, 40%);">-             LOGP(DTRX, LOGL_ERROR, "cmdname buffer too small %lu >= %lu\n",</span><br><span style="color: hsl(0, 100%, 40%);">-                    p - buf_in, cmdname_len);</span><br><span style="color: hsl(120, 100%, 40%);">+     if (p - buf_in >= sizeof(rsp->cmd)) {</span><br><span style="color: hsl(120, 100%, 40%);">+           LOGP(DTRX, LOGL_ERROR, "cmd buffer too small %lu >= %lu\n",</span><br><span style="color: hsl(120, 100%, 40%);">+                      p - buf_in, sizeof(rsp->cmd));</span><br><span>            goto parse_err;</span><br><span>      }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-   cmdname_out[0] = '\0';</span><br><span style="color: hsl(0, 100%, 40%);">-  strncat(cmdname_out, buf_in + 4, p - buf_in - 4);</span><br><span style="color: hsl(120, 100%, 40%);">+     rsp->cmd[0] = '\0';</span><br><span style="color: hsl(120, 100%, 40%);">+        strncat(rsp->cmd, buf_in + 4, p - buf_in - 4);</span><br><span> </span><br><span>        /* Now comes the status code of the response */</span><br><span>      p++;</span><br><span style="color: hsl(0, 100%, 40%);">-    if (sscanf(p, "%d", status) != 1)</span><br><span style="color: hsl(120, 100%, 40%);">+   if (sscanf(p, "%d", &rsp->status) != 1)</span><br><span>             goto parse_err;</span><br><span> </span><br><span>  /* Now copy back the parameters */</span><br><span>@@ -401,13 +406,13 @@</span><br><span>   else</span><br><span>                 k = p + strlen(p);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-  if (strlen(k) >= params_len) {</span><br><span style="color: hsl(120, 100%, 40%);">+     if (strlen(k) >= sizeof(rsp->params)) {</span><br><span>                LOGP(DTRX, LOGL_ERROR, "params buffer too small %lu >= %lu\n",</span><br><span style="color: hsl(0, 100%, 40%);">-                     strlen(k), params_len);</span><br><span style="color: hsl(120, 100%, 40%);">+                       strlen(k), sizeof(rsp->params));</span><br><span>          goto parse_err;</span><br><span>      }</span><br><span style="color: hsl(0, 100%, 40%);">-       params_out[0] = '\0';</span><br><span style="color: hsl(0, 100%, 40%);">-   strcat(params_out, k);</span><br><span style="color: hsl(120, 100%, 40%);">+        rsp->params[0] = '\0';</span><br><span style="color: hsl(120, 100%, 40%);">+     strcat(rsp->params, k);</span><br><span>   return 0;</span><br><span> </span><br><span> parse_err:</span><br><span>@@ -416,15 +421,15 @@</span><br><span>  return -1;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-static bool cmd_matches_rsp(struct trx_ctrl_msg *tcm, char *rspname, char* params)</span><br><span style="color: hsl(120, 100%, 40%);">+static bool cmd_matches_rsp(struct trx_ctrl_msg *tcm, struct trx_ctrl_rsp *rsp)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">-     if (strcmp(tcm->cmd, rspname))</span><br><span style="color: hsl(120, 100%, 40%);">+     if (strcmp(tcm->cmd, rsp->cmd))</span><br><span>                return false;</span><br><span> </span><br><span>    /* For SETSLOT we also need to check if it's the response for the</span><br><span>           specific timeslot. For other commands such as SETRXGAIN, it is</span><br><span>       expected that they can return different values */</span><br><span style="color: hsl(0, 100%, 40%);">-    if (strcmp(tcm->cmd, "SETSLOT") == 0 && strcmp(tcm->params, params))</span><br><span style="color: hsl(120, 100%, 40%);">+  if (strcmp(tcm->cmd, "SETSLOT") == 0 && strcmp(tcm->params, rsp->params))</span><br><span>                 return false;</span><br><span> </span><br><span>    return true;</span><br><span>@@ -435,8 +440,9 @@</span><br><span> {</span><br><span>      struct trx_l1h *l1h = ofd->data;</span><br><span>  struct phy_instance *pinst = l1h->phy_inst;</span><br><span style="color: hsl(0, 100%, 40%);">-  char buf[1500], cmdname[50], params[100];</span><br><span style="color: hsl(0, 100%, 40%);">-       int len, resp;</span><br><span style="color: hsl(120, 100%, 40%);">+        char buf[1500];</span><br><span style="color: hsl(120, 100%, 40%);">+       struct trx_ctrl_rsp rsp;</span><br><span style="color: hsl(120, 100%, 40%);">+      int len;</span><br><span>     struct trx_ctrl_msg *tcm;</span><br><span> </span><br><span>        len = recv(ofd->fd, buf, sizeof(buf) - 1, 0);</span><br><span>@@ -444,7 +450,7 @@</span><br><span>               return len;</span><br><span>  buf[len] = '\0';</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-    if (parse_rsp(buf, len, cmdname, sizeof(cmdname), params, sizeof(params), &resp) < 0)</span><br><span style="color: hsl(120, 100%, 40%);">+  if (parse_rsp(buf, len, &rsp) < 0)</span><br><span>            return 0;</span><br><span> </span><br><span>        LOGP(DTRX, LOGL_INFO, "Response message: '%s'\n", buf);</span><br><span>@@ -456,7 +462,7 @@</span><br><span>      /* get command for response message */</span><br><span>       if (llist_empty(&l1h->trx_ctrl_list)) {</span><br><span>               /* RSP from a retransmission, skip it */</span><br><span style="color: hsl(0, 100%, 40%);">-                if (l1h->last_acked && cmd_matches_rsp(l1h->last_acked, cmdname, params)) {</span><br><span style="color: hsl(120, 100%, 40%);">+             if (l1h->last_acked && cmd_matches_rsp(l1h->last_acked, &rsp)) {</span><br><span>                   LOGP(DTRX, LOGL_NOTICE, "Discarding duplicated RSP "</span><br><span>                               "from old CMD '%s'\n", buf);</span><br><span>                       return 0;</span><br><span>@@ -469,9 +475,9 @@</span><br><span>              list);</span><br><span> </span><br><span>   /* check if response matches command */</span><br><span style="color: hsl(0, 100%, 40%);">- if (!cmd_matches_rsp(tcm, cmdname, params)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!cmd_matches_rsp(tcm, &rsp)) {</span><br><span>               /* RSP from a retransmission, skip it */</span><br><span style="color: hsl(0, 100%, 40%);">-                if (l1h->last_acked && cmd_matches_rsp(l1h->last_acked, cmdname, params)) {</span><br><span style="color: hsl(120, 100%, 40%);">+             if (l1h->last_acked && cmd_matches_rsp(l1h->last_acked, &rsp)) {</span><br><span>                   LOGP(DTRX, LOGL_NOTICE, "Discarding duplicated RSP "</span><br><span>                               "from old CMD '%s'\n", buf);</span><br><span>                       return 0;</span><br><span>@@ -484,7 +490,7 @@</span><br><span>      }</span><br><span> </span><br><span>        /* check for response code */</span><br><span style="color: hsl(0, 100%, 40%);">-   if (resp) {</span><br><span style="color: hsl(120, 100%, 40%);">+   if (rsp.status) {</span><br><span>            LOGP(DTRX, (tcm->critical) ? LOGL_FATAL : LOGL_NOTICE,</span><br><span>                    "transceiver (%s) rejected TRX command "</span><br><span>                   "with response: '%s'\n",</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/11765">change 11765</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/11765"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: osmo-bts </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: Icb84bce0621042afa4301678ba1cc58d8e3662bb </div>
<div style="display:none"> Gerrit-Change-Number: 11765 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: Pau Espin Pedrol <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Harald Welte <laforge@gnumonks.org> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder (1000002) </div>
<div style="display:none"> Gerrit-Reviewer: Pau Espin Pedrol <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-CC: Vadim Yanitskiy <axilirator@gmail.com> </div>