pespin has submitted this change. ( https://gerrit.osmocom.org/c/libosmo-sigtran/+/43753?usp=email ) Change subject: ss7_asp_peer: Improvements in ss7_asp_peer_snprintf() ...................................................................... ss7_asp_peer: Improvements in ss7_asp_peer_snprintf() Convert it to osmo_strbuf, which simplifies the logic of the function, taking osmo_multiaddr_ip_and_port_snprintf() as reference to implement it. As a result: * Single IPv6 addresses now get properly enclosed in [], eg: "[::]:2905". * We avoid passing NULL to snprintf(), which is unspecified on some platforms. Change-Id: I115951627adb0c6778a9a36159237c3eea130300 --- M src/ss7_asp_peer.c M tests/ss7/ss7_test.err M tests/ss7/ss7_test.ok 3 files changed, 31 insertions(+), 30 deletions(-) Approvals: fixeria: Looks good to me, but someone else must approve osmith: Looks good to me, approved Jenkins Builder: Verified diff --git a/src/ss7_asp_peer.c b/src/ss7_asp_peer.c index c31cdd9..2fd4c63 100644 --- a/src/ss7_asp_peer.c +++ b/src/ss7_asp_peer.c @@ -53,35 +53,36 @@ int ss7_asp_peer_snprintf(char *buf, size_t buf_len, struct osmo_ss7_asp_peer *peer) { - int len = 0, offset = 0, rem = buf_len; - int ret, i; - char *after; + struct osmo_strbuf sb = { .buf = buf, .len = buf_len }; + bool is_v6 = false; + unsigned int i; char *primary; - if (buf_len < 3) - return -EINVAL; - - if (peer->host_cnt > 1) { - ret = snprintf(buf, rem, "("); - if (ret < 0) - return ret; - OSMO_SNPRINTF_RET(ret, rem, offset, len); + if (peer->host_cnt == 0) { + OSMO_STRBUF_PRINTF(sb, "NULL:%u", peer->port); + return sb.chars_needed; } - for (i = 0; i < peer->host_cnt; i++) { + + if (peer->host_cnt > 1) + OSMO_STRBUF_PRINTF(sb, "("); + else if (peer->host[0] && (is_v6 = !!strchr(peer->host[0], ':'))) + OSMO_STRBUF_PRINTF(sb, "["); /* IPv6, add [] to separate from port. */ + + for (i = 0; i < peer->host_cnt - 1; i++) { primary = (peer->idx_primary == i) ? "*" : ""; - if (peer->host_cnt == 1) - after = ""; - else - after = (i == (peer->host_cnt - 1)) ? ")" : "|"; - ret = snprintf(buf + offset, rem, "%s%s%s", peer->host[i] ? : "0.0.0.0", primary, after); - OSMO_SNPRINTF_RET(ret, rem, offset, len); + OSMO_STRBUF_PRINTF(sb, "%s%s|", peer->host[i] ? : "NULL", primary); } - ret = snprintf(buf + offset, rem, ":%u", peer->port); - if (ret < 0) - return ret; - OSMO_SNPRINTF_RET(ret, rem, offset, len); + primary = (peer->idx_primary == i) ? "*" : ""; + OSMO_STRBUF_PRINTF(sb, "%s%s", peer->host[i] ? : "NULL", primary); - return len; + if (peer->host_cnt > 1) + OSMO_STRBUF_PRINTF(sb, ")"); + else if (is_v6) + OSMO_STRBUF_PRINTF(sb, "]"); + + OSMO_STRBUF_PRINTF(sb, ":%u", peer->port); + + return sb.chars_needed; } /*! \brief Set (copy) addresses for a given ASP peer. Previous addresses are freed. diff --git a/tests/ss7/ss7_test.err b/tests/ss7/ss7_test.err index 4d2e308..7333bb6 100644 --- a/tests/ss7/ss7_test.err +++ b/tests/ss7/ss7_test.err @@ -44,13 +44,13 @@ DLSS7 DEBUG XUA_AS(as1){AS_DOWN}: Allocated DLSS7 INFO 0: AS(as1) Created AS DLSS7 INFO 0: AS(as1) Adding ASP asp1 to AS -DLSS7 INFO 0: ASP(asp1) Restarting ASP asp1, r=:0<->l=:2905 +DLSS7 INFO 0: ASP(asp1) Restarting ASP asp1, r=NULL:0<->l=NULL:2905 DLSS7 DEBUG xua_default_lm(asp1){IDLE}: Allocated DLSS7 DEBUG XUA_ASP(asp1){ASP_DOWN}: Allocated DLINP ERROR CLICONN(asp1,){CLOSED} connect: socket creation error (-22) DLINP DEBUG CLICONN(asp1,){CLOSED} close() DLINP INFO CLICONN(asp1,){WAIT_RECONNECT} retrying reconnect in 5 seconds... -DLSS7 ERROR 0: ASP(asp1) Unable to open stream client for ASP asp1, :2905 ==> :0 +DLSS7 ERROR 0: ASP(asp1) Unable to open stream client for ASP asp1, NULL:2905 ==> NULL:0 DLSS7 DEBUG XUA_ASP(asp1){ASP_DOWN}: Received Event M-ASP_UP.req DLSS7 DEBUG 0: ASP(asp1) Tx 8 bytes: [L2]> 01 00 03 01 00 00 00 08 DLSS7 ERROR 0: ASP(asp1) Cannot transmit, asp->client not connected diff --git a/tests/ss7/ss7_test.ok b/tests/ss7/ss7_test.ok index 532d6f0..b8e7d00 100644 --- a/tests/ss7/ss7_test.ok +++ b/tests/ss7/ss7_test.ok @@ -28,13 +28,13 @@ route with partial mask: 0.1.0/11 route with non-consecutive mask: 0.1.0/7.255.1 Testing SS7 linkset/link -ss7_asp_peer_snprintf: :0 -ss7_asp_peer_snprintf: :2905 +ss7_asp_peer_snprintf: NULL:0 +ss7_asp_peer_snprintf: NULL:2905 ss7_asp_peer_snprintf: (127.0.0.1*|::1):2905 ss7_asp_peer_snprintf: (127.0.0.1*|::1):2905 ss7_asp_peer_snprintf: (127.0.0.1|::1):2905 ss7_asp_peer_snprintf: (127.0.0.1|::1*):2905 -ss7_asp_peer_snprintf: ::1*:2905 -ss7_asp_peer_snprintf: :2905 +ss7_asp_peer_snprintf: [::1*]:2905 +ss7_asp_peer_snprintf: NULL:2905 ss7_asp_peer_snprintf: 0.0.0.0:2905 -ss7_asp_peer_snprintf: :::2905 +ss7_asp_peer_snprintf: [::]:2905 -- To view, visit https://gerrit.osmocom.org/c/libosmo-sigtran/+/43753?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: libosmo-sigtran Gerrit-Branch: master Gerrit-Change-Id: I115951627adb0c6778a9a36159237c3eea130300 Gerrit-Change-Number: 43753 Gerrit-PatchSet: 5 Gerrit-Owner: pespin <pespin@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy@sysmocom.de> Gerrit-Reviewer: jolly <andreas@eversberg.eu> Gerrit-Reviewer: osmith <osmith@sysmocom.de> Gerrit-Reviewer: pespin <pespin@sysmocom.de>