<p>Max <strong>merged</strong> this change.</p><p><a href="https://gerrit.osmocom.org/12318">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Jenkins Builder: Verified
daniel: Looks good to me, approved
osmith: Looks good to me, but someone else must approve
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">ctrl: log host/port on errors<br><br>In case of multiple ctrl-client entries in .cfg file it's impossible to<br>see which one is causing particular ctrl error. Fix this by introducing<br>macro wrapper for stderr logging which always show host:port relevant to<br>the error.<br><br>Change-Id: I788d51359965a66c54075a3971aa7824c3bfb0bf<br>Related: SYS#2655<br>---<br>M src/simple_ctrl.c<br>1 file changed, 24 insertions(+), 16 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/simple_ctrl.c b/src/simple_ctrl.c</span><br><span>index 2261323..b56fc83 100644</span><br><span>--- a/src/simple_ctrl.c</span><br><span>+++ b/src/simple_ctrl.c</span><br><span>@@ -37,6 +37,9 @@</span><br><span> </span><br><span> #include "simple_ctrl.h"</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+#define CTRL_ERR(cfg, fmt, args...) \</span><br><span style="color: hsl(120, 100%, 40%);">+ fprintf(stderr, "CTRL %s:%u error: " fmt, cfg.remote_host, cfg.remote_port, ##args)</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> /***********************************************************************</span><br><span> * blocking I/O with timeout helpers</span><br><span> ***********************************************************************/</span><br><span>@@ -98,20 +101,28 @@</span><br><span> int fd;</span><br><span> uint32_t next_id;</span><br><span> uint32_t tout_msec;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct ctrl_cfg cfg;</span><br><span> };</span><br><span> </span><br><span> struct simple_ctrl_handle *simple_ctrl_open(void *ctx, const char *host, uint16_t dport,</span><br><span> uint32_t tout_msec)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- struct simple_ctrl_handle *sch;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct simple_ctrl_handle *sch = talloc_zero(ctx, struct simple_ctrl_handle);</span><br><span> fd_set writeset;</span><br><span> int off = 0;</span><br><span> int rc, fd;</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ if (!sch)</span><br><span style="color: hsl(120, 100%, 40%);">+ return NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ sch->cfg.name = talloc_strdup(sch, "simple-ctrl");</span><br><span style="color: hsl(120, 100%, 40%);">+ sch->cfg.remote_host = talloc_strdup(sch, host);</span><br><span style="color: hsl(120, 100%, 40%);">+ sch->cfg.remote_port = dport;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> fd = osmo_sock_init(AF_INET, SOCK_STREAM, IPPROTO_TCP, host, dport,</span><br><span> OSMO_SOCK_F_CONNECT | OSMO_SOCK_F_NONBLOCK);</span><br><span> if (fd < 0) {</span><br><span style="color: hsl(0, 100%, 40%);">- fprintf(stderr, "CTRL: error connecting socket: %s\n", strerror(errno));</span><br><span style="color: hsl(120, 100%, 40%);">+ CTRL_ERR(sch->cfg, "connecting socket: %s\n", strerror(errno));</span><br><span> return NULL;</span><br><span> }</span><br><span> </span><br><span>@@ -120,23 +131,20 @@</span><br><span> FD_SET(fd, &writeset);</span><br><span> rc = select(fd+1, NULL, &writeset, NULL, timeval_from_msec(tout_msec));</span><br><span> if (rc == 0) {</span><br><span style="color: hsl(0, 100%, 40%);">- fprintf(stderr, "CTRL: timeout during connect\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ CTRL_ERR(sch->cfg, "timeout during connect\n");</span><br><span> goto out_close;</span><br><span> }</span><br><span> if (rc < 0) {</span><br><span style="color: hsl(0, 100%, 40%);">- fprintf(stderr, "CTRL: error connecting socket: %s\n", strerror(errno));</span><br><span style="color: hsl(120, 100%, 40%);">+ CTRL_ERR(sch->cfg, "error connecting socket: %s\n", strerror(errno));</span><br><span> goto out_close;</span><br><span> }</span><br><span> </span><br><span> /* set FD blocking again */</span><br><span> if (ioctl(fd, FIONBIO, (unsigned char *)&off) < 0) {</span><br><span style="color: hsl(0, 100%, 40%);">- fprintf(stderr, "CTRL: cannot set socket blocking: %s\n", strerror(errno));</span><br><span style="color: hsl(120, 100%, 40%);">+ CTRL_ERR(sch->cfg, "cannot set socket blocking: %s\n", strerror(errno));</span><br><span> goto out_close;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- sch = talloc_zero(ctx, struct simple_ctrl_handle);</span><br><span style="color: hsl(0, 100%, 40%);">- if (!sch)</span><br><span style="color: hsl(0, 100%, 40%);">- goto out_close;</span><br><span> sch->fd = fd;</span><br><span> sch->tout_msec = tout_msec;</span><br><span> return sch;</span><br><span>@@ -165,10 +173,10 @@</span><br><span> </span><br><span> rc = read_timeout(sch->fd, (uint8_t *) &hh, sizeof(hh), sch->tout_msec);</span><br><span> if (rc < 0) {</span><br><span style="color: hsl(0, 100%, 40%);">- fprintf(stderr, "CTRL: Error during read: %d\n", rc);</span><br><span style="color: hsl(120, 100%, 40%);">+ CTRL_ERR(sch->cfg, "read(): %d\n", rc);</span><br><span> return NULL;</span><br><span> } else if (rc < sizeof(hh)) {</span><br><span style="color: hsl(0, 100%, 40%);">- fprintf(stderr, "CTRL: ERROR: short read (header)\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ CTRL_ERR(sch->cfg, "short read (header)\n");</span><br><span> return NULL;</span><br><span> }</span><br><span> len = ntohs(hh.len);</span><br><span>@@ -182,7 +190,7 @@</span><br><span> resp->l2h = resp->tail;</span><br><span> rc = read(sch->fd, resp->l2h, len);</span><br><span> if (rc < len) {</span><br><span style="color: hsl(0, 100%, 40%);">- fprintf(stderr, "CTRL: ERROR: short read (payload)\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ CTRL_ERR(sch->cfg, "short read (payload)\n");</span><br><span> msgb_free(resp);</span><br><span> return NULL;</span><br><span> }</span><br><span>@@ -214,7 +222,7 @@</span><br><span> *tmp = '\0';</span><br><span> return resp;</span><br><span> } else {</span><br><span style="color: hsl(0, 100%, 40%);">- fprintf(stderr, "unknown IPA message %s\n", msgb_hexdump(resp));</span><br><span style="color: hsl(120, 100%, 40%);">+ CTRL_ERR(sch->cfg, "unknown IPA message %s\n", msgb_hexdump(resp));</span><br><span> msgb_free(resp);</span><br><span> }</span><br><span> }</span><br><span>@@ -229,10 +237,10 @@</span><br><span> </span><br><span> rc = write_timeout(sch->fd, msg->data, msg->len, sch->tout_msec);</span><br><span> if (rc < 0) {</span><br><span style="color: hsl(0, 100%, 40%);">- fprintf(stderr, "CTRL: Error during write: %d\n", rc);</span><br><span style="color: hsl(120, 100%, 40%);">+ CTRL_ERR(sch->cfg, "write(): %d\n", rc);</span><br><span> return rc;</span><br><span> } else if (rc < msg->len) {</span><br><span style="color: hsl(0, 100%, 40%);">- fprintf(stderr, "CTRL: ERROR: short write\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ CTRL_ERR(sch->cfg, "short write\n");</span><br><span> msgb_free(msg);</span><br><span> return -1;</span><br><span> } else {</span><br><span>@@ -283,7 +291,7 @@</span><br><span> free(rx_var);</span><br><span> free(rx_val);</span><br><span> } else {</span><br><span style="color: hsl(0, 100%, 40%);">- fprintf(stderr, "CTRL: ERROR: GET(%s) results in '%s'\n", var, (char *)msgb_l2(resp));</span><br><span style="color: hsl(120, 100%, 40%);">+ CTRL_ERR(sch->cfg, "GET(%s) results in '%s'\n", var, (char *)msgb_l2(resp));</span><br><span> }</span><br><span> </span><br><span> msgb_free(resp);</span><br><span>@@ -321,7 +329,7 @@</span><br><span> free(rx_var);</span><br><span> }</span><br><span> } else {</span><br><span style="color: hsl(0, 100%, 40%);">- fprintf(stderr, "CTRL: ERROR: SET(%s=%s) results in '%s'\n", var, val, (char *) msgb_l2(resp));</span><br><span style="color: hsl(120, 100%, 40%);">+ CTRL_ERR(sch->cfg, "SET(%s=%s) results in '%s'\n", var, val, (char *) msgb_l2(resp));</span><br><span> }</span><br><span> </span><br><span> msgb_free(resp);</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/12318">change 12318</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/12318"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmo-sysmon </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: I788d51359965a66c54075a3971aa7824c3bfb0bf </div>
<div style="display:none"> Gerrit-Change-Number: 12318 </div>
<div style="display:none"> Gerrit-PatchSet: 7 </div>
<div style="display:none"> Gerrit-Owner: Max <msuraev@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: Max <msuraev@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: daniel <dwillmann@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: osmith <osmith@sysmocom.de> </div>
<div style="display:none"> Gerrit-CC: Pau Espin Pedrol <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-CC: Stefan Sperling <stsp@stsp.name> </div>