From 9ce5bf90bb9b5ca24f1276e263261278f6d712ed Mon Sep 17 00:00:00 2001
From: Max <msuraev@sysmocom.de>
Date: Fri, 28 Dec 2018 19:28:03 +0100
Subject: [PATCH] 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
---
 src/simple_ctrl.c | 40 ++++++++++++++++++++++++----------------
 1 file changed, 24 insertions(+), 16 deletions(-)

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 @@ struct simple_ctrl_handle {
 	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 @@ struct simple_ctrl_handle *simple_ctrl_open(void *ctx, const char *host, uint16_
 	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 @@ static struct msgb *simple_ipa_receive(struct simple_ctrl_handle *sch)
 
 	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 @@ static struct msgb *simple_ipa_receive(struct simple_ctrl_handle *sch)
 	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 @@ struct msgb *simple_ctrl_receive(struct simple_ctrl_handle *sch)
 			*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 @@ static int simple_ctrl_send(struct simple_ctrl_handle *sch, struct msgb *msg)
 
 	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 @@ char *simple_ctrl_get(struct simple_ctrl_handle *sch, const char *var)
 		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 @@ int simple_ctrl_set(struct simple_ctrl_handle *sch, const char *var, const char
 			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);