[M] Change in libosmocore[master]: socket: Introduce API osmo_sock_multiaddr_get_name_buf()
laforge has submitted this change. (
https://gerrit.osmocom.org/c/libosmocore/+/35180?usp=email )
Change subject: socket: Introduce API osmo_sock_multiaddr_get_name_buf()
..
socket: Introduce API osmo_sock_multiaddr_get_name_buf()
An extra osmo_multiaddr_ip_and_port_snprintf() API is introduced which
is used by osmo_sock_multiaddr_get_name_buf() but which will also be
used by other app uers willing to use
osmo_sock_multiaddr_get_ip_and_port() according to its needs (eg. only
printing the local side).
Related: SYS#6636
Change-Id: I48950754ed6f61ee5ffa04a447fab8903f10acc0
---
M TODO-RELEASE
M include/osmocom/core/socket.h
M src/core/libosmocore.map
M src/core/socket.c
4 files changed, 129 insertions(+), 1 deletion(-)
Approvals:
Jenkins Builder: Verified
laforge: Looks good to me, approved
osmith: Looks good to me, but someone else must approve
diff --git a/TODO-RELEASE b/TODO-RELEASE
index e365746..316c0ec 100644
--- a/TODO-RELEASE
+++ b/TODO-RELEASE
@@ -8,7 +8,7 @@
# If any interfaces have been removed or changed since the last public
release: c:r:0.
#library whatdescription / commit summary line
core ADD osmo_sock_multiaddr_{add,del}_local_addr()
-core ADD osmo_sock_multiaddr_get_ip_and_port()
+core ADD osmo_sock_multiaddr_get_ip_and_port(),
osmo_multiaddr_ip_and_port_snprintf(), osmo_sock_multiaddr_get_name_buf()
core ADD gsmtap_inst_fd2() core, DEPRECATE gsmtap_inst_fd()
isdn ABI change add states and flags for external T200
handling
gsmABI change add T200 timer states to lapdm_datalink
diff --git a/include/osmocom/core/socket.h b/include/osmocom/core/socket.h
index b8b04f8..dd14556 100644
--- a/include/osmocom/core/socket.h
+++ b/include/osmocom/core/socket.h
@@ -193,6 +193,10 @@
int osmo_sock_multiaddr_get_ip_and_port(int fd, int ip_proto, char *ip, size_t
*ip_cnt, size_t ip_len,
char *port, size_t port_len, bool
local);
+int osmo_multiaddr_ip_and_port_snprintf(char *str, size_t str_len,
+ const char *ip, size_t ip_cnt, size_t
ip_len,
+ const char *portbuf);
+int osmo_sock_multiaddr_get_name_buf(char *str, size_t str_len, int fd, int
sk_proto);
int osmo_sock_multiaddr_add_local_addr(int sfd, const char **addrs, size_t
addrs_cnt);
int osmo_sock_multiaddr_del_local_addr(int sfd, const char **addrs, size_t
addrs_cnt);
diff --git a/src/core/libosmocore.map b/src/core/libosmocore.map
index ffbbd2f..e7daced 100644
--- a/src/core/libosmocore.map
+++ b/src/core/libosmocore.map
@@ -298,6 +298,7 @@
osmo_macaddr_parse;
osmo_mnl_destroy;
osmo_mnl_init;
+osmo_multiaddr_ip_and_port_snprintf;
osmo_netdev_add_addr;
osmo_netdev_add_route;
osmo_netdev_alloc;
@@ -437,6 +438,7 @@
osmo_sock_multiaddr_add_local_addr;
osmo_sock_multiaddr_del_local_addr;
osmo_sock_multiaddr_get_ip_and_port;
+osmo_sock_multiaddr_get_name_buf;
osmo_sock_set_dscp;
osmo_sock_set_priority;
osmo_sock_unix_init;
diff --git a/src/core/socket.c b/src/core/socket.c
index 51703ef..c600732 100644
--- a/src/core/socket.c
+++ b/src/core/socket.c
@@ -2018,6 +2018,112 @@
return talloc_asprintf(ctx, "(%s)", str);
}
+/*! Format multiple IP addresses and/or port number into a combined string
buffer
+ * \param[out] str Destination string buffer.
+ * \param[in] str_len sizeof(str).
+ * \param[out] ip Pointer to memory holding ip_cnt consecutive buffers of
size ip_len.
+ * \param[out] ip_cnt length ip array pointer. on return it contains the
number of addresses found.
+ * \param[in] ip_len length of each of the string buffer in the the ip array.
+ * \param[out] port number (will be printed in when not NULL).
+ * \return String length as returned by snprintf(), or negative on error.
+ *
+ * This API expectes an ip array as the one filled in by
+ * osmo_sock_multiaddr_get_ip_and_port(), and hence it's a good companion for
+ * that API.
+ */
+int osmo_multiaddr_ip_and_port_snprintf(char *str, size_t str_len,
+ const char *ip, size_t ip_cnt, size_t
ip_len,
+ const char *portbuf)
+{
+ struct osmo_strbuf sb = { .buf = str, .len = str_len };
+ bool is_v6 = false;
+ unsigned int i;
+
+ if (ip_cnt == 0) {
+ OSMO_STRBUF_PRINTF(sb, "NULL:%s", portbuf);
+ return sb.chars_needed;
+ }
+ if (ip_cnt > 1)
+ OSMO_STRBUF_PRINTF(sb, "(");
+ else if ((is_v6 = !!strchr([0], ':'))) /* IPv6, add [] to separate
from port. */
+ OSMO_STRBUF_PRINTF(sb, "[");
+
+ for (i = 0; i < ip_cnt - 1; i++)
+ OSMO_STRBUF_PRINTF(sb, "%s|", [i * ip_len]);
+ OSMO_STRBUF_PRINTF(sb, "%s", [i * ip_len]);
+
+ if (ip_cnt > 1)
+
[M] Change in libosmocore[master]: socket: Introduce API osmo_sock_multiaddr_get_name_buf()
Attention is currently required from: neels, pespin. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35180?usp=email ) Change subject: socket: Introduce API osmo_sock_multiaddr_get_name_buf() .. Patch Set 5: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35180?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I48950754ed6f61ee5ffa04a447fab8903f10acc0 Gerrit-Change-Number: 35180 Gerrit-PatchSet: 5 Gerrit-Owner: pespin Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: osmith Gerrit-CC: neels Gerrit-Attention: neels Gerrit-Attention: pespin Gerrit-Comment-Date: Thu, 07 Dec 2023 13:23:03 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in libosmocore[master]: socket: Introduce API osmo_sock_multiaddr_get_name_buf()
Attention is currently required from: neels, pespin. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35180?usp=email ) Change subject: socket: Introduce API osmo_sock_multiaddr_get_name_buf() .. Patch Set 5: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35180?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I48950754ed6f61ee5ffa04a447fab8903f10acc0 Gerrit-Change-Number: 35180 Gerrit-PatchSet: 5 Gerrit-Owner: pespin Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: osmith Gerrit-CC: neels Gerrit-Attention: neels Gerrit-Attention: pespin Gerrit-Comment-Date: Thu, 07 Dec 2023 13:20:48 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in libosmocore[master]: socket: Introduce API osmo_sock_multiaddr_get_name_buf()
Attention is currently required from: laforge, neels, pespin. osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35180?usp=email ) Change subject: socket: Introduce API osmo_sock_multiaddr_get_name_buf() .. Patch Set 5: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35180?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I48950754ed6f61ee5ffa04a447fab8903f10acc0 Gerrit-Change-Number: 35180 Gerrit-PatchSet: 5 Gerrit-Owner: pespin Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: osmith Gerrit-CC: neels Gerrit-Attention: neels Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Comment-Date: Thu, 07 Dec 2023 09:49:26 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in libosmocore[master]: socket: Introduce API osmo_sock_multiaddr_get_name_buf()
Attention is currently required from: laforge, neels, osmith, pespin. Hello Jenkins Builder, laforge, osmith, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/35180?usp=email to look at the new patch set (#5). The following approvals got outdated and were removed: Code-Review+1 by osmith, Verified+1 by Jenkins Builder Change subject: socket: Introduce API osmo_sock_multiaddr_get_name_buf() .. socket: Introduce API osmo_sock_multiaddr_get_name_buf() An extra osmo_multiaddr_ip_and_port_snprintf() API is introduced which is used by osmo_sock_multiaddr_get_name_buf() but which will also be used by other app uers willing to use osmo_sock_multiaddr_get_ip_and_port() according to its needs (eg. only printing the local side). Related: SYS#6636 Change-Id: I48950754ed6f61ee5ffa04a447fab8903f10acc0 --- M TODO-RELEASE M include/osmocom/core/socket.h M src/core/libosmocore.map M src/core/socket.c 4 files changed, 129 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/80/35180/5 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35180?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I48950754ed6f61ee5ffa04a447fab8903f10acc0 Gerrit-Change-Number: 35180 Gerrit-PatchSet: 5 Gerrit-Owner: pespin Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: osmith Gerrit-CC: neels Gerrit-Attention: osmith Gerrit-Attention: neels Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-MessageType: newpatchset
[M] Change in libosmocore[master]: socket: Introduce API osmo_sock_multiaddr_get_name_buf()
Attention is currently required from: laforge, neels, pespin. osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35180?usp=email ) Change subject: socket: Introduce API osmo_sock_multiaddr_get_name_buf() .. Patch Set 4: Code-Review+1 (2 comments) File src/core/socket.c: https://gerrit.osmocom.org/c/libosmocore/+/35180/comment/006df9a7_0ed4e9d1 PS1, Line 1968: return -EBADF; > Ack Done https://gerrit.osmocom.org/c/libosmocore/+/35180/comment/d2157ef8_64affc27 PS1, Line 2005: OSMO_STRBUF_PRINTF(sb, ""); > I could write an entire novel explaining the error there, but have that token > printed and looking at […] I think Pau's reasoning is fine here. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35180?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I48950754ed6f61ee5ffa04a447fab8903f10acc0 Gerrit-Change-Number: 35180 Gerrit-PatchSet: 4 Gerrit-Owner: pespin Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: osmith Gerrit-CC: neels Gerrit-Attention: neels Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Comment-Date: Wed, 06 Dec 2023 14:35:03 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: neels Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[M] Change in libosmocore[master]: socket: Introduce API osmo_sock_multiaddr_get_name_buf()
Attention is currently required from: laforge, neels. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35180?usp=email ) Change subject: socket: Introduce API osmo_sock_multiaddr_get_name_buf() .. Patch Set 4: (1 comment) File src/core/socket.c: https://gerrit.osmocom.org/c/libosmocore/+/35180/comment/55043613_6059b39a PS2, Line 1970: char hostbuf[32][INET6_ADDRSTRLEN]; > This should be s/32/OSMO_SOCK_MAX_ADDRS/g Done -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35180?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I48950754ed6f61ee5ffa04a447fab8903f10acc0 Gerrit-Change-Number: 35180 Gerrit-PatchSet: 4 Gerrit-Owner: pespin Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-CC: neels Gerrit-Attention: neels Gerrit-Attention: laforge Gerrit-Comment-Date: Wed, 06 Dec 2023 14:08:34 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[M] Change in libosmocore[master]: socket: Introduce API osmo_sock_multiaddr_get_name_buf()
Attention is currently required from: laforge, neels, pespin. Hello Jenkins Builder, laforge, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/35180?usp=email to look at the new patch set (#4). The following approvals got outdated and were removed: Verified+1 by Jenkins Builder Change subject: socket: Introduce API osmo_sock_multiaddr_get_name_buf() .. socket: Introduce API osmo_sock_multiaddr_get_name_buf() Change-Id: I48950754ed6f61ee5ffa04a447fab8903f10acc0 --- M TODO-RELEASE M include/osmocom/core/socket.h M src/core/libosmocore.map M src/core/socket.c 4 files changed, 107 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/80/35180/4 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35180?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I48950754ed6f61ee5ffa04a447fab8903f10acc0 Gerrit-Change-Number: 35180 Gerrit-PatchSet: 4 Gerrit-Owner: pespin Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-CC: neels Gerrit-Attention: neels Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-MessageType: newpatchset
[M] Change in libosmocore[master]: socket: Introduce API osmo_sock_multiaddr_get_name_buf()
Attention is currently required from: laforge, neels, pespin. Hello Jenkins Builder, laforge, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/35180?usp=email to look at the new patch set (#3). The following approvals got outdated and were removed: Code-Review+1 by laforge, Verified+1 by Jenkins Builder Change subject: socket: Introduce API osmo_sock_multiaddr_get_name_buf() .. socket: Introduce API osmo_sock_multiaddr_get_name_buf() Change-Id: I48950754ed6f61ee5ffa04a447fab8903f10acc0 --- M TODO-RELEASE M include/osmocom/core/socket.h M src/core/libosmocore.map M src/core/socket.c 4 files changed, 107 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/80/35180/3 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35180?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I48950754ed6f61ee5ffa04a447fab8903f10acc0 Gerrit-Change-Number: 35180 Gerrit-PatchSet: 3 Gerrit-Owner: pespin Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-CC: neels Gerrit-Attention: neels Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-MessageType: newpatchset
[M] Change in libosmocore[master]: socket: Introduce API osmo_sock_multiaddr_get_name_buf()
Attention is currently required from: neels, pespin. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35180?usp=email ) Change subject: socket: Introduce API osmo_sock_multiaddr_get_name_buf() .. Patch Set 2: Code-Review+1 (2 comments) File src/core/socket.c: https://gerrit.osmocom.org/c/libosmocore/+/35180/comment/69741a2f_dea5ba34 PS1, Line 1993: v6 = !!s > I only care about checking if the address is ipv6 in the case where we only > have 1 IP, because in th […] Done https://gerrit.osmocom.org/c/libosmocore/+/35180/comment/3d4b51f2_63325c1c PS1, Line 2052: int osmo_sock_get_name_buf(char *str, size_t str_len, int fd) > I prefer keeping the multiaddr prefix instead of a "2", we already use it in > several places. […] agreeing with separate api and agreeing with multiaddr. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35180?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I48950754ed6f61ee5ffa04a447fab8903f10acc0 Gerrit-Change-Number: 35180 Gerrit-PatchSet: 2 Gerrit-Owner: pespin Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-CC: neels Gerrit-Attention: neels Gerrit-Attention: pespin Gerrit-Comment-Date: Mon, 04 Dec 2023 16:38:49 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: neels Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[M] Change in libosmocore[master]: socket: Introduce API osmo_sock_multiaddr_get_name_buf()
Attention is currently required from: neels. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35180?usp=email ) Change subject: socket: Introduce API osmo_sock_multiaddr_get_name_buf() .. Patch Set 2: (1 comment) File src/core/socket.c: https://gerrit.osmocom.org/c/libosmocore/+/35180/comment/c50c3f0a_96acc67c PS2, Line 1970: char hostbuf[32][INET6_ADDRSTRLEN]; This should be s/32/OSMO_SOCK_MAX_ADDRS/g -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35180?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I48950754ed6f61ee5ffa04a447fab8903f10acc0 Gerrit-Change-Number: 35180 Gerrit-PatchSet: 2 Gerrit-Owner: pespin Gerrit-Reviewer: Jenkins Builder Gerrit-CC: laforge Gerrit-CC: neels Gerrit-Attention: neels Gerrit-Comment-Date: Mon, 04 Dec 2023 15:32:45 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
[M] Change in libosmocore[master]: socket: Introduce API osmo_sock_multiaddr_get_name_buf()
Attention is currently required from: neels. Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/35180?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Verified+1 by Jenkins Builder Change subject: socket: Introduce API osmo_sock_multiaddr_get_name_buf() .. socket: Introduce API osmo_sock_multiaddr_get_name_buf() Change-Id: I48950754ed6f61ee5ffa04a447fab8903f10acc0 --- M TODO-RELEASE M include/osmocom/core/socket.h M src/core/libosmocore.map M src/core/socket.c 4 files changed, 107 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/80/35180/2 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35180?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I48950754ed6f61ee5ffa04a447fab8903f10acc0 Gerrit-Change-Number: 35180 Gerrit-PatchSet: 2 Gerrit-Owner: pespin Gerrit-Reviewer: Jenkins Builder Gerrit-CC: laforge Gerrit-CC: neels Gerrit-Attention: neels Gerrit-MessageType: newpatchset
[M] Change in libosmocore[master]: socket: Introduce API osmo_sock_multiaddr_get_name_buf()
Attention is currently required from: neels. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35180?usp=email ) Change subject: socket: Introduce API osmo_sock_multiaddr_get_name_buf() .. Patch Set 1: (5 comments) File src/core/socket.c: https://gerrit.osmocom.org/c/libosmocore/+/35180/comment/ec1feec8_ff7c2583 PS1, Line 1968: return -EBADF; > i understand, but for the snprintf() like function signature, if at all > possible you should always r […] Ack https://gerrit.osmocom.org/c/libosmocore/+/35180/comment/1411b96c_cca4890e PS1, Line 1993: v6 = !!s > assigning a value to is_v6 in an 'else' clause and using is_v6 below. […] I only care about checking if the address is ipv6 in the case where we only have 1 IP, because in that case I need to put "[]" around it to separate it from the port number. If there's more than 1, no need, because everything is already enclosed in "()". So by doing it here I avoid doing a lookup in a string unless it's strictly needed. https://gerrit.osmocom.org/c/libosmocore/+/35180/comment/489652e3_5bf0d19e PS1, Line 2005: OSMO_STRBUF_PRINTF(sb, ""); > (could print this above instead of adding the need_more_bufs local var, just > my opinion) […] I could write an entire novel explaining the error there, but have that token printed and looking at the code already quickly explains what's going on. I want to print it here because at least is provides as many addresses as it can fit. https://gerrit.osmocom.org/c/libosmocore/+/35180/comment/ad4d2b6d_0d053a6e PS1, Line 2019: bool is_v6 = false; > (can this code dup be collapsed with the above, maybe with a static func?) It could, I'd need to pass the sb as a param and so on, and I'm not sure if the macros can use an sb pointer https://gerrit.osmocom.org/c/libosmocore/+/35180/comment/4852c8d4_24954207 PS1, Line 2052: int osmo_sock_get_name_buf(char *str, size_t str_len, int fd) > my first thought is "yes, sounds good". […] I prefer keeping the multiaddr prefix instead of a "2", we already use it in several places. This way code knowing only to be handling single address protocols can keep using it. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35180?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I48950754ed6f61ee5ffa04a447fab8903f10acc0 Gerrit-Change-Number: 35180 Gerrit-PatchSet: 1 Gerrit-Owner: pespin Gerrit-Reviewer: Jenkins Builder Gerrit-CC: laforge Gerrit-CC: neels Gerrit-Attention: neels Gerrit-Comment-Date: Fri, 01 Dec 2023 15:33:59 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[M] Change in libosmocore[master]: socket: Introduce API osmo_sock_multiaddr_get_name_buf()
Attention is currently required from: pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35180?usp=email ) Change subject: socket: Introduce API osmo_sock_multiaddr_get_name_buf() .. Patch Set 1: (5 comments) File src/core/socket.c: https://gerrit.osmocom.org/c/libosmocore/+/35180/comment/2ad85b55_eccf6410 PS1, Line 1968: return -EBADF; i understand, but for the snprintf() like function signature, if at all possible you should always return sb.chars_needed. Otherwise, if anyone calls OSMO_STRBUF_APPEND(sb, osmo_sock_multiaddr_get_name_buf, ...), they don't get "" returned, in the expected place; instead, it looks like OSMO_STRBUF_APPEND() might even crash? [the place handling < 0 there looks a bit weird; same as insufficient buffer, which should be a different case.] the function's purpose is to print a string, not to validate an fd, so better just do this here: OSMO_STRBUF_PRINTF(sb, ""); return sb.chars_needed; because then the caller will get the error output in the right place, like, contrived example: Stats: reads: 0 writes: 0 address: When you return an error, the string composition may abort and omit the "Stats:..." part too in the resulting output. (as i read on, i see that osmo_sock_get_name_buf() makes the same mistake, i guess i need to fix that) https://gerrit.osmocom.org/c/libosmocore/+/35180/comment/64fdb01a_d46bb32b PS1, Line 1993: v6 = !!s assigning a value to is_v6 in an 'else' clause and using is_v6 below. It looks like the implication is that if num_hostbuf <= 1, then it cannot be a v6 address, which sounds odd to me https://gerrit.osmocom.org/c/libosmocore/+/35180/comment/08a76724_bd72f529 PS1, Line 2005: OSMO_STRBUF_PRINTF(sb, ""); (could print this above instead of adding the need_more_bufs local var, just my opinion) (also i find it weird to print in user output, it makes no sense to the reader, because it is reporting an error in internal buffer sizes, and not about the user's config or current status of resources; a user may think that a RAM upgrade is necessary to run osmocom, wildly wrong) https://gerrit.osmocom.org/c/libosmocore/+/35180/comment/accd4083_8c902ac9 PS1, Line 2019: bool is_v6 = false; (can this code dup be collapsed with the above, maybe with a static func?) https://gerrit.osmocom.org/c/libosmocore/+/35180/comment/0dcd0bb5_dbc45421 PS1, Line 2052: int osmo_sock_get_name_buf(char *str, size_t str_len, int fd) > Something which we could do is internally integrating the multiaddr support > into the existing genera […] my first thought is "yes, sounds good". Do you think though there may be situations where we don't want all of the addresses in, say, all of the logging or vty output all the time, where instead we want to just continue printing the first address? I guess that is hard to answer, so unfortunately the best backwards compatible way is to only have the separate new API returning multiple addrs. Callers can switch to the new function at their own time. But, hey, how about we call the new function osmo_sock_get_name_buf2(), as in, v2 supports multiple addresses, and everyone should just use that everywhere from now on? -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35180?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I48950754ed6f61ee5ffa04a447fab8903f10acc0 Gerrit-Change-Number: 35180 Gerrit-PatchSet: 1 Gerrit-Owner: pespin Gerrit-Reviewer: Jenkins Builder Gerrit-CC: laforge Gerrit-CC: neels Gerrit-Attention: pespin Gerrit-Comment-Date: Thu, 30 Nov 2023 23:39:27 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[M] Change in libosmocore[master]: socket: Introduce API osmo_sock_multiaddr_get_name_buf()
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35180?usp=email ) Change subject: socket: Introduce API osmo_sock_multiaddr_get_name_buf() .. Patch Set 1: (1 comment) File src/core/socket.c: https://gerrit.osmocom.org/c/libosmocore/+/35180/comment/2e83f24a_61d0386d PS1, Line 2052: int osmo_sock_get_name_buf(char *str, size_t str_len, int fd) Something which we could do is internally integrating the multiaddr support into the existing general osmo_sock_get_name_buf(), by means of doing inside here: int sock_type; socklen_t sock_type_length = sizeof(sock_type); getsockopt(socket, SOL_SOCKET, SO_TYPE, _type, _type_length); if (sock_type == IPPROTO_SCTP) osmo_sock_multiaddr_get_name_buf(); I'm fine with both approaches, the existing one or the one presented in this commit. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35180?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I48950754ed6f61ee5ffa04a447fab8903f10acc0 Gerrit-Change-Number: 35180 Gerrit-PatchSet: 1 Gerrit-Owner: pespin Gerrit-CC: Jenkins Builder Gerrit-CC: laforge Gerrit-Comment-Date: Thu, 30 Nov 2023 19:16:32 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
[M] Change in libosmocore[master]: socket: Introduce API osmo_sock_multiaddr_get_name_buf()
pespin has uploaded this change for review. (
https://gerrit.osmocom.org/c/libosmocore/+/35180?usp=email )
Change subject: socket: Introduce API osmo_sock_multiaddr_get_name_buf()
..
socket: Introduce API osmo_sock_multiaddr_get_name_buf()
Change-Id: I48950754ed6f61ee5ffa04a447fab8903f10acc0
---
M TODO-RELEASE
M include/osmocom/core/socket.h
M src/core/libosmocore.map
M src/core/socket.c
4 files changed, 107 insertions(+), 1 deletion(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/80/35180/1
diff --git a/TODO-RELEASE b/TODO-RELEASE
index e365746..9db6f16 100644
--- a/TODO-RELEASE
+++ b/TODO-RELEASE
@@ -8,7 +8,7 @@
# If any interfaces have been removed or changed since the last public
release: c:r:0.
#library whatdescription / commit summary line
core ADD osmo_sock_multiaddr_{add,del}_local_addr()
-core ADD osmo_sock_multiaddr_get_ip_and_port()
+core ADD osmo_sock_multiaddr_get_ip_and_port(),
osmo_sock_multiaddr_get_name_buf()
core ADD gsmtap_inst_fd2() core, DEPRECATE gsmtap_inst_fd()
isdn ABI change add states and flags for external T200
handling
gsmABI change add T200 timer states to lapdm_datalink
diff --git a/include/osmocom/core/socket.h b/include/osmocom/core/socket.h
index 9397343..cae8b11 100644
--- a/include/osmocom/core/socket.h
+++ b/include/osmocom/core/socket.h
@@ -193,6 +193,7 @@
int osmo_sock_multiaddr_get_ip_and_port(int fd, int ip_proto, char *ip, size_t
ip_len, size_t *ip_cnt,
char *port, size_t port_len, bool
local);
+int osmo_sock_multiaddr_get_name_buf(char *str, size_t str_len, int fd, int
sk_proto);
int osmo_sock_multiaddr_add_local_addr(int sfd, const char **addrs, size_t
addrs_cnt);
int osmo_sock_multiaddr_del_local_addr(int sfd, const char **addrs, size_t
addrs_cnt);
diff --git a/src/core/libosmocore.map b/src/core/libosmocore.map
index 1d00fe8..e67946b 100644
--- a/src/core/libosmocore.map
+++ b/src/core/libosmocore.map
@@ -437,6 +437,7 @@
osmo_sock_multiaddr_add_local_addr;
osmo_sock_multiaddr_del_local_addr;
osmo_sock_multiaddr_get_ip_and_port;
+osmo_sock_multiaddr_get_name_buf;
osmo_sock_set_dscp;
osmo_sock_set_priority;
osmo_sock_unix_init;
diff --git a/src/core/socket.c b/src/core/socket.c
index 1d97018..2f3a02b 100644
--- a/src/core/socket.c
+++ b/src/core/socket.c
@@ -1952,6 +1952,101 @@
* \param[out] str Destination string buffer.
* \param[in] str_len sizeof(str).
* \param[in] fd File descriptor of socket.
+ * \param[in] fd IPPROTO of the socket, eg: IPPROTO_SCTP.
+ * \return String length as returned by snprintf(), or negative on error.
+ */
+int osmo_sock_multiaddr_get_name_buf(char *str, size_t str_len, int fd, int
sk_proto)
+{
+ char hostbuf[32][INET6_ADDRSTRLEN];
+ size_t num_hostbuf = ARRAY_SIZE(hostbuf);
+ char portbuf[6];
+ struct osmo_strbuf sb = { .buf = str, .len = str_len };
+ unsigned int i;
+
+ if (fd < 0) {
+ osmo_strlcpy(str, "", str_len);
+ return -EBADF;
+ }
+
+ switch (sk_proto) {
+ case IPPROTO_SCTP:
+ break; /* continue below */
+ default:
+ return osmo_sock_get_name_buf(str, str_len, fd);
+ }
+
+ /* get remote */
+ OSMO_STRBUF_PRINTF(sb, "r=");
+ if (osmo_sock_multiaddr_get_ip_and_port(fd, sk_proto, [0][0],
sizeof(hostbuf[0]),
+ _hostbuf, portbuf,
sizeof(portbuf), false) != 0) {
+ OSMO_STRBUF_PRINTF(sb, "NULL");
+ } else if (num_hostbuf == 0) {
+ OSMO_STRBUF_PRINTF(sb, "NULL");
+ } else {
+ bool is_v6 = false;
+ bool need_more_bufs = num_hostbuf > ARRAY_SIZE(hostbuf);
+ if (need_more_bufs)
+ num_hostbuf = ARRAY_SIZE(hostbuf);
+
+ if (num_hostbuf > 1)
+ OSMO_STRBUF_PRINTF(sb, "(");
+ else if ((is_v6 = !!strchr(hostbuf[0], ':'))) /* IPv6, add []
to separate from port. */
+ OSMO_STRBUF_PRINTF(sb, "[");
+
+ for (i = 0; i < num_hostbuf - 1; i++)
+ OSMO_STRBUF_PRINTF(sb, "%s,", hostbuf[i]);
+ OSMO_STRBUF_PRINTF(sb, "%s", hostbuf[i]);
+
+ if (num_hostbuf > 1)
+ OSMO_STRBUF_PRINTF(sb, ")");
+ else if (is_v6)
+ OSMO_STRBUF_PRINTF(sb, "]");
+ if (need_more_bufs)
+ OSMO_STRBUF_PRINTF(sb, "");
+ OSMO_STRBUF_PRINTF(sb, ":%s", portbuf);
+ }
+
+ OSMO_STRBUF_PRINTF(sb, "<->l=");
+
+ /* get local */
+ num_hostbuf = ARRAY_SIZE(hostbuf);
+ if (osmo_sock_multiaddr_get_ip_and_port(fd, sk_proto, [0][0],
