<p>Pau Espin Pedrol has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/11765">View Change</a></p><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;">git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/65/11765/1</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: newchange </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: 1 </div>
<div style="display:none"> Gerrit-Owner: Pau Espin Pedrol <pespin@sysmocom.de> </div>