Change in osmo-msc[master]: GSUP: include terminating nul in inter-MSC source/destination name
Neels Hofmeyr has submitted this change and it was merged. (
https://gerrit.osmocom.org/13612 )
Change subject: GSUP: include terminating nul in inter-MSC source/destination
name
..
GSUP: include terminating nul in inter-MSC source/destination name
Before, I was testing with osmo-hlr patch
I01a45900e14d41bcd338f50ad85d9fabf2c61405 applied, but that patch is currently
in an abandoned state.
This is the counterpart implemented in osmo-msc: always include the terminating
nul char in the "blob" that is the MSC IPA name.
The dualities in the formats of routing between MSCs is whether to handle it as
a char*, or as a uint8_t* with explicit len (a blob).
In the VTY config to indicate target MSCs for inter-MSC handover, we have
strings. We currently even completely lack a way of configuring any blob-like
data as a VTY config item.
In osmo-hlr, the IPA names used for routing are currently received as a char*
which *includes* the terminating nul char. So in osmo-msc, if we also always
include the nul char, it works.
Instead, we could just send the char* part without the nul char, and apply
above mentioned osmo-hlr patch. That patch would magically match a name that
lacks a nul with a name that includes one. I think it is better to agree on one
format on the GSUP wire now, instead of making assumptions in osmo-hlr on the
format of the source/target names for routing. This format, from the way GSUP
so far transmits the IPA SERNO tag when a client attaches to osmo-hlr, happens
to include the terminating nul char.
Change-Id: I9ca8c9eef104519ed1ea46e2fef46dcdc0d554eb
---
M src/libmsc/e_link.c
1 file changed, 14 insertions(+), 7 deletions(-)
Approvals:
Jenkins Builder: Verified
Harald Welte: Looks good to me, approved
diff --git a/src/libmsc/e_link.c b/src/libmsc/e_link.c
index 685ca7c..0a2be79 100644
--- a/src/libmsc/e_link.c
+++ b/src/libmsc/e_link.c
@@ -68,6 +68,7 @@
{
struct e_link *e;
struct msc_role_common *c = msc_role->priv;
+ size_t use_len;
/* use msub as talloc parent, so we can move an e_link from msc_t to
msc_i when it is established. */
e = talloc_zero(c->msub, struct e_link);
@@ -76,13 +77,19 @@
e->gcm = gcm;
- /* Expecting all code paths to print the remote name according to
remote_name_len. To be paranoid, place a nul
-* character after the end. */
- e->remote_name = talloc_size(e, remote_name_len + 1);
+ /* FIXME: this is a braindamaged duality of char* and blob, which we
can't seem to get rid of easily.
+* See also osmo-hlr change I01a45900e14d41bcd338f50ad85d9fabf2c61405
which resolved this on the
+* osmo-hlr side, but was abandoned. Not sure which way is the right
solution. */
+ /* To be able to include a terminating NUL character when sending the
IPA name, append one if there is none yet.
+* Current osmo-hlr needs the terminating NUL to be included. */
+ use_len = remote_name_len;
+ if (remote_name[use_len-1] != '\0')
+ use_len++;
+ e->remote_name = talloc_size(e, use_len);
OSMO_ASSERT(e->remote_name);
memcpy(e->remote_name, remote_name, remote_name_len);
- e->remote_name[remote_name_len] = '\0';
- e->remote_name_len = remote_name_len;
+ e->remote_name[use_len-1] = '\0';
+ e->remote_name_len = use_len;
e_link_assign(e, msc_role);
return e;
@@ -123,9 +130,9 @@
*gsup_msg = (struct osmo_gsup_message){
.message_class = OSMO_GSUP_MESSAGE_CLASS_INTER_MSC,
.source_name = (const uint8_t*)local_msc_name,
- .source_name_len = strlen(local_msc_name),
+ .source_name_len = strlen(local_msc_name)+1, /* include
terminating nul */
.destination_name = (const uint8_t*)e->remote_name,
- .destination_name_len = e->remote_name_len,
+ .destination_name_len = e->remote_name_len, /* the nul here is
also included, from e_link_alloc() */
};
if (vsub)
--
To view, visit https://gerrit.osmocom.org/13612
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I9ca8c9eef104519ed1ea46e2fef46dcdc0d554eb
Gerrit-Change-Number: 13612
Gerrit-PatchSet: 10
Gerrit-Owner: Neels Hofmeyr
Gerrit-Reviewer: Harald Welte
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr
Change in osmo-msc[master]: GSUP: include terminating nul in inter-MSC source/destination name
Harald Welte has posted comments on this change. ( https://gerrit.osmocom.org/13612 ) Change subject: GSUP: include terminating nul in inter-MSC source/destination name .. Patch Set 8: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/13612 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9ca8c9eef104519ed1ea46e2fef46dcdc0d554eb Gerrit-Change-Number: 13612 Gerrit-PatchSet: 8 Gerrit-Owner: Neels Hofmeyr Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Comment-Date: Wed, 08 May 2019 06:31:36 + Gerrit-HasComments: No Gerrit-HasLabels: Yes
Change in osmo-msc[master]: GSUP: include terminating nul in inter-MSC source/destination name
Hello Harald Welte, Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/13612 to look at the new patch set (#4). Change subject: GSUP: include terminating nul in inter-MSC source/destination name .. GSUP: include terminating nul in inter-MSC source/destination name Before, I was testing with osmo-hlr patch I01a45900e14d41bcd338f50ad85d9fabf2c61405 applied, but that patch is currently in an abandoned state. This is the counterpart implemented in osmo-msc: always include the terminating nul char in the "blob" that is the MSC IPA name. The dualities in the formats of routing between MSCs is whether to handle it as a char*, or as a uint8_t* with explicit len (a blob). In the VTY config to indicate target MSCs for inter-MSC handover, we have strings. We currently even completely lack a way of configuring any blob-like data as a VTY config item. In osmo-hlr, the IPA names used for routing are currently received as a char* which *includes* the terminating nul char. So in osmo-msc, if we also always include the nul char, it works. Instead, we could just send the char* part without the nul char, and apply above mentioned osmo-hlr patch. That patch would magically match a name that lacks a nul with a name that includes one. I think it is better to agree on one format on the GSUP wire now, instead of making assumptions in osmo-hlr on the format of the source/target names for routing. This format, from the way GSUP so far transmits the IPA SERNO tag when a client attaches to osmo-hlr, happens to include the terminating nul char. Change-Id: I9ca8c9eef104519ed1ea46e2fef46dcdc0d554eb --- M src/libmsc/e_link.c 1 file changed, 14 insertions(+), 7 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/12/13612/4 -- To view, visit https://gerrit.osmocom.org/13612 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9ca8c9eef104519ed1ea46e2fef46dcdc0d554eb Gerrit-Change-Number: 13612 Gerrit-PatchSet: 4 Gerrit-Owner: Neels Hofmeyr Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder (102)
Change in osmo-msc[master]: GSUP: include terminating nul in inter-MSC source/destination name
Harald Welte has posted comments on this change. ( https://gerrit.osmocom.org/13612 ) Change subject: GSUP: include terminating nul in inter-MSC source/destination name .. Patch Set 2: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/13612 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9ca8c9eef104519ed1ea46e2fef46dcdc0d554eb Gerrit-Change-Number: 13612 Gerrit-PatchSet: 2 Gerrit-Owner: Neels Hofmeyr Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Comment-Date: Sun, 14 Apr 2019 15:31:02 + Gerrit-HasComments: No Gerrit-HasLabels: Yes
Change in osmo-msc[master]: GSUP: include terminating nul in inter-MSC source/destination name
Neels Hofmeyr has uploaded this change for review. (
https://gerrit.osmocom.org/13612
Change subject: GSUP: include terminating nul in inter-MSC source/destination
name
..
GSUP: include terminating nul in inter-MSC source/destination name
Before, I was testing with osmo-hlr patch
I01a45900e14d41bcd338f50ad85d9fabf2c61405 applied, but that patch is currently
in an abandoned state.
This is the counterpart implemented in osmo-msc: always include the terminating
nul char in the "blob" that is the MSC IPA name.
The dualities in the formats of routing between MSCs is whether to handle it as
a char*, or as a uint8_t* with explicit len (a blob).
In the VTY config to indicate target MSCs for inter-MSC handover, we have
strings. We currently even completely lack a way of configuring any blob-like
data as a VTY config item.
In osmo-hlr, the IPA names used for routing are currently received as a char*
which *includes* the terminating nul char. So in osmo-msc, if we also always
include the nul char, it works.
Instead, we could just send the char* part without the nul char, and apply
above mentioned osmo-hlr patch. That patch would magically match a name that
lacks a nul with a name that includes one. I think it is better to agree on one
format on the GSUP wire now, instead of making assumptions in osmo-hlr on the
format of the source/target names for routing. This format, from the way GSUP
so far transmits the IPA SERNO tag when a client attaches to osmo-hlr, happens
to include the terminating nul char.
Change-Id: I9ca8c9eef104519ed1ea46e2fef46dcdc0d554eb
---
M src/libmsc/e_link.c
1 file changed, 14 insertions(+), 7 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/12/13612/1
diff --git a/src/libmsc/e_link.c b/src/libmsc/e_link.c
index c0ebbf6..1ec3647 100644
--- a/src/libmsc/e_link.c
+++ b/src/libmsc/e_link.c
@@ -69,6 +69,7 @@
{
struct e_link *e;
struct msc_role_common *c = msc_role->priv;
+ size_t use_len;
/* use msub as talloc parent, so we can move an e_link from msc_t to
msc_i when it is established. */
e = talloc_zero(c->msub, struct e_link);
@@ -79,12 +80,18 @@
.gcm = gcm,
};
- /* Expecting all code paths to print the remote name according to
remote_name_len. To be paranoid, place a nul
-* character after the end. */
- e->remote_name = talloc_size(e, remote_name_len + 1);
+ /* FIXME: this is a braindamaged duality of char* and blob, which we
can't seem to get rid of easily.
+* See also osmo-hlr change I01a45900e14d41bcd338f50ad85d9fabf2c61405
which resolved this on the
+* osmo-hlr side, but was abandoned. Not sure which way is the right
solution. */
+ /* To be able to include a terminating NUL character when sending the
IPA name, append one if there is none yet.
+* Current osmo-hlr needs the terminating NUL to be included. */
+ use_len = remote_name_len;
+ if (remote_name[use_len-1] != '\0')
+ use_len++;
+ e->remote_name = talloc_size(e, use_len);
memcpy(e->remote_name, remote_name, remote_name_len);
- e->remote_name[remote_name_len] = '\0';
- e->remote_name_len = remote_name_len;
+ e->remote_name[use_len-1] = '\0';
+ e->remote_name_len = use_len;
e_link_assign(e, msc_role);
return e;
@@ -125,9 +132,9 @@
*gsup_msg = (struct osmo_gsup_message){
.message_class = OSMO_GSUP_MESSAGE_CLASS_INTER_MSC,
.source_name = (const uint8_t*)local_msc_name,
- .source_name_len = strlen(local_msc_name),
+ .source_name_len = strlen(local_msc_name)+1, /* include
terminating nul */
.destination_name = (const uint8_t*)e->remote_name,
- .destination_name_len = e->remote_name_len,
+ .destination_name_len = e->remote_name_len, /* the nul here is
also included, from e_link_alloc() */
};
if (vsub)
--
To view, visit https://gerrit.osmocom.org/13612
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9ca8c9eef104519ed1ea46e2fef46dcdc0d554eb
Gerrit-Change-Number: 13612
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr
