summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDaniel Willmann <dwillmann@sysmocom.de>2014-01-17 15:17:36 +0100
committerDaniel Willmann <daniel@totalueberwachung.de>2014-01-28 11:18:51 +0100
commitc0ee00451e9504dbf2ffdfe14d12fb6873a333a3 (patch)
tree771706cbc0c187d35be280a92631a6eadf326efd
parentdcf38e1c9623f997d9b87c1a5868862581aa05cf (diff)
smpp_smsc: Fix socket read() error handling
Read returning -1 is an error here so make sure to print the actual reason and close the socket. Before this patch we just looped over the fd with read returning -1 every time. EINTR is handled to not cause an error and we don't need to check EAGAIN/EWOULDBLOCK since the callback is only called in case there is something to read. To avoid copy&paste issues the check is implemented as a macro and the log message moved into a separate if.
-rw-r--r--openbsc/src/libmsc/smpp_smsc.c47
1 files changed, 31 insertions, 16 deletions
diff --git a/openbsc/src/libmsc/smpp_smsc.c b/openbsc/src/libmsc/smpp_smsc.c
index 64ed2005f..1e9829bae 100644
--- a/openbsc/src/libmsc/smpp_smsc.c
+++ b/openbsc/src/libmsc/smpp_smsc.c
@@ -762,6 +762,23 @@ static int smpp_pdu_rx(struct osmo_esme *esme, struct msgb *msg __uses)
return rc;
}
+/* This macro should be called after a call to read() in the read_cb of an
+ * osmo_fd to properly check for errors.
+ * rc is the return value of read, err_label is the label to jump to in case of
+ * an error. The code there should handle closing the connection.
+ * FIXME: This code should go in libosmocore utils.h so it can be used by other
+ * projects as well.
+ * */
+#define OSMO_FD_CHECK_READ(rc, err_label) \
+ if (rc < 0) { \
+ /* EINTR is a non-fatal error, just try again */ \
+ if (errno == EINTR) \
+ return 0; \
+ goto err_label; \
+ } else if (rc == 0) { \
+ goto err_label; \
+ }
+
/* !\brief call-back when per-ESME TCP socket has some data to be read */
static int esme_link_read_cb(struct osmo_fd *ofd)
{
@@ -777,13 +794,13 @@ static int esme_link_read_cb(struct osmo_fd *ofd)
case READ_ST_IN_LEN:
rdlen = sizeof(uint32_t) - esme->read_idx;
rc = read(ofd->fd, lenptr + esme->read_idx, rdlen);
- if (rc < 0) {
- LOGP(DSMPP, LOGL_ERROR, "[%s] read returned %d\n",
- esme->system_id, rc);
- } else if (rc == 0) {
- goto dead_socket;
- } else
- esme->read_idx += rc;
+ if (rc < 0)
+ LOGP(DSMPP, LOGL_ERROR, "[%s] read returned %d (%s)\n",
+ esme->system_id, rc, strerror(errno));
+ OSMO_FD_CHECK_READ(rc, dead_socket);
+
+ esme->read_idx += rc;
+
if (esme->read_idx >= sizeof(uint32_t)) {
esme->read_len = ntohl(len);
msg = msgb_alloc(esme->read_len, "SMPP Rx");
@@ -800,15 +817,13 @@ static int esme_link_read_cb(struct osmo_fd *ofd)
msg = esme->read_msg;
rdlen = esme->read_len - esme->read_idx;
rc = read(ofd->fd, msg->tail, OSMO_MIN(rdlen, msgb_tailroom(msg)));
- if (rc < 0) {
- LOGP(DSMPP, LOGL_ERROR, "[%s] read returned %d\n",
- esme->system_id, rc);
- } else if (rc == 0) {
- goto dead_socket;
- } else {
- esme->read_idx += rc;
- msgb_put(msg, rc);
- }
+ if (rc < 0)
+ LOGP(DSMPP, LOGL_ERROR, "[%s] read returned %d (%s)\n",
+ esme->system_id, rc, strerror(errno));
+ OSMO_FD_CHECK_READ(rc, dead_socket);
+
+ esme->read_idx += rc;
+ msgb_put(msg, rc);
if (esme->read_idx >= esme->read_len) {
rc = smpp_pdu_rx(esme, esme->read_msg);