Max has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/12318 )

Change subject: ctrl: log host/port on errors
......................................................................

ctrl: log host/port on errors

In case of multiple ctrl-client entries in .cfg file it's impossible to
see which one is causing particular ctrl error. Fix this by introducing
macro wrapper for stderr logging which always show host:port relevant to
the error.

Change-Id: I788d51359965a66c54075a3971aa7824c3bfb0bf
Related: SYS#2655
---
M src/simple_ctrl.c
1 file changed, 24 insertions(+), 16 deletions(-)

Approvals:
  Jenkins Builder: Verified
  daniel: Looks good to me, approved
  osmith: Looks good to me, but someone else must approve



diff --git a/src/simple_ctrl.c b/src/simple_ctrl.c
index 2261323..b56fc83 100644
--- a/src/simple_ctrl.c
+++ b/src/simple_ctrl.c
@@ -37,6 +37,9 @@

 #include "simple_ctrl.h"

+#define CTRL_ERR(cfg, fmt, args...) \
+       fprintf(stderr, "CTRL %s:%u error: " fmt, cfg.remote_host, 
cfg.remote_port, ##args)
+
 /***********************************************************************
  * blocking I/O with timeout helpers
  ***********************************************************************/
@@ -98,20 +101,28 @@
        int fd;
        uint32_t next_id;
        uint32_t tout_msec;
+       struct ctrl_cfg cfg;
 };

 struct simple_ctrl_handle *simple_ctrl_open(void *ctx, const char *host, 
uint16_t dport,
                                            uint32_t tout_msec)
 {
-       struct simple_ctrl_handle *sch;
+       struct simple_ctrl_handle *sch = talloc_zero(ctx, struct 
simple_ctrl_handle);
        fd_set writeset;
        int off = 0;
        int rc, fd;

+       if (!sch)
+               return NULL;
+
+       sch->cfg.name = talloc_strdup(sch, "simple-ctrl");
+       sch->cfg.remote_host = talloc_strdup(sch, host);
+       sch->cfg.remote_port = dport;
+
        fd = osmo_sock_init(AF_INET, SOCK_STREAM, IPPROTO_TCP, host, dport,
                            OSMO_SOCK_F_CONNECT | OSMO_SOCK_F_NONBLOCK);
        if (fd < 0) {
-               fprintf(stderr, "CTRL: error connecting socket: %s\n", 
strerror(errno));
+               CTRL_ERR(sch->cfg, "connecting socket: %s\n", strerror(errno));
                return NULL;
        }

@@ -120,23 +131,20 @@
        FD_SET(fd, &writeset);
        rc = select(fd+1, NULL, &writeset, NULL, timeval_from_msec(tout_msec));
        if (rc == 0) {
-               fprintf(stderr, "CTRL: timeout during connect\n");
+               CTRL_ERR(sch->cfg, "timeout during connect\n");
                goto out_close;
        }
        if (rc < 0) {
-               fprintf(stderr, "CTRL: error connecting socket: %s\n", 
strerror(errno));
+               CTRL_ERR(sch->cfg, "error connecting socket: %s\n", 
strerror(errno));
                goto out_close;
        }

        /* set FD blocking again */
        if (ioctl(fd, FIONBIO, (unsigned char *)&off) < 0) {
-               fprintf(stderr, "CTRL: cannot set socket blocking: %s\n", 
strerror(errno));
+               CTRL_ERR(sch->cfg, "cannot set socket blocking: %s\n", 
strerror(errno));
                goto out_close;
        }

-       sch = talloc_zero(ctx, struct simple_ctrl_handle);
-       if (!sch)
-               goto out_close;
        sch->fd = fd;
        sch->tout_msec = tout_msec;
        return sch;
@@ -165,10 +173,10 @@

        rc = read_timeout(sch->fd, (uint8_t *) &hh, sizeof(hh), sch->tout_msec);
        if (rc < 0) {
-               fprintf(stderr, "CTRL: Error during read: %d\n", rc);
+               CTRL_ERR(sch->cfg, "read(): %d\n", rc);
                return NULL;
        } else if (rc < sizeof(hh)) {
-               fprintf(stderr, "CTRL: ERROR: short read (header)\n");
+               CTRL_ERR(sch->cfg, "short read (header)\n");
                return NULL;
        }
        len = ntohs(hh.len);
@@ -182,7 +190,7 @@
        resp->l2h = resp->tail;
        rc = read(sch->fd, resp->l2h, len);
        if (rc < len) {
-               fprintf(stderr, "CTRL: ERROR: short read (payload)\n");
+               CTRL_ERR(sch->cfg, "short read (payload)\n");
                msgb_free(resp);
                return NULL;
        }
@@ -214,7 +222,7 @@
                        *tmp = '\0';
                        return resp;
                } else {
-                       fprintf(stderr, "unknown IPA message %s\n", 
msgb_hexdump(resp));
+                       CTRL_ERR(sch->cfg, "unknown IPA message %s\n", 
msgb_hexdump(resp));
                        msgb_free(resp);
                }
        }
@@ -229,10 +237,10 @@

        rc = write_timeout(sch->fd, msg->data, msg->len, sch->tout_msec);
        if (rc < 0) {
-               fprintf(stderr, "CTRL: Error during write: %d\n", rc);
+               CTRL_ERR(sch->cfg, "write(): %d\n", rc);
                return rc;
        } else if (rc < msg->len) {
-               fprintf(stderr, "CTRL: ERROR: short write\n");
+               CTRL_ERR(sch->cfg, "short write\n");
                msgb_free(msg);
                return -1;
        } else {
@@ -283,7 +291,7 @@
                free(rx_var);
                free(rx_val);
        } else {
-               fprintf(stderr, "CTRL: ERROR: GET(%s) results in '%s'\n", var, 
(char *)msgb_l2(resp));
+               CTRL_ERR(sch->cfg, "GET(%s) results in '%s'\n", var, (char 
*)msgb_l2(resp));
        }

        msgb_free(resp);
@@ -321,7 +329,7 @@
                        free(rx_var);
                }
        } else {
-               fprintf(stderr, "CTRL: ERROR: SET(%s=%s) results in '%s'\n", 
var, val, (char *) msgb_l2(resp));
+               CTRL_ERR(sch->cfg, "SET(%s=%s) results in '%s'\n", var, val, 
(char *) msgb_l2(resp));
        }

        msgb_free(resp);

--
To view, visit https://gerrit.osmocom.org/12318
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-sysmon
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I788d51359965a66c54075a3971aa7824c3bfb0bf
Gerrit-Change-Number: 12318
Gerrit-PatchSet: 7
Gerrit-Owner: Max <[email protected]>
Gerrit-Reviewer: Harald Welte <[email protected]>
Gerrit-Reviewer: Jenkins Builder (1000002)
Gerrit-Reviewer: Max <[email protected]>
Gerrit-Reviewer: daniel <[email protected]>
Gerrit-Reviewer: osmith <[email protected]>
Gerrit-CC: Pau Espin Pedrol <[email protected]>
Gerrit-CC: Stefan Sperling <[email protected]>

Reply via email to