openbsc[master]: libmsc: Fix wrong handling of user_message_reference parameter

[Harald Welte]

Patch Set 2: Code-Review+2

-- 
To view, visit https://gerrit.osmocom.org/3554
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If748548a4a223e529a1110c89e483b599b406e8b
Gerrit-PatchSet: 2
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pablo Neira Ayuso 
Gerrit-HasComments: No

[MERGED] openbsc[master]: libmsc: Fix wrong handling of user_message_reference parameter

[Harald Welte]

Harald Welte has submitted this change and it was merged.

Change subject: libmsc: Fix wrong handling of user_message_reference parameter
..


libmsc: Fix wrong handling of user_message_reference parameter

libsmpp34 already converts received TLV integer values to native
endianess in libsmpp34_(un)pack.

Converting them again at receive time swaps the 2 bytes of
user_message_reference, then using a wrong value. As GSM03.40 spec
uses only 1 byte for the id, then only the high byte of the initial
value is used and eventually sent back to the ESME. Again, at that time,
htons() is not needed because libsmpp34 already handles that part.

See OS-#2429 for more details.

Change-Id: If748548a4a223e529a1110c89e483b599b406e8b
---
M openbsc/src/libmsc/smpp_openbsc.c
1 file changed, 2 insertions(+), 2 deletions(-)

Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified
  Pablo Neira Ayuso: Looks good to me, but someone else must approve



diff --git a/openbsc/src/libmsc/smpp_openbsc.c 
b/openbsc/src/libmsc/smpp_openbsc.c
index e656376..af2d1be 100644
--- a/openbsc/src/libmsc/smpp_openbsc.c
+++ b/openbsc/src/libmsc/smpp_openbsc.c
@@ -120,7 +120,7 @@
}
break;
case TLVID_user_message_reference:
-   msg_ref = ntohs(t->value.val16);
+   msg_ref = t->value.val16;
break;
default:
break;
@@ -436,7 +436,7 @@
memset(, 0, sizeof(tlv));
tlv.tag = tag;
tlv.length = 2;
-   tlv.value.val16 = htons(val);
+   tlv.value.val16 = val;
build_tlv(req_tlv, );
 }
 

-- 
To view, visit https://gerrit.osmocom.org/3554
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: If748548a4a223e529a1110c89e483b599b406e8b
Gerrit-PatchSet: 2
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pablo Neira Ayuso 

openbsc[master]: libmsc: Fix wrong handling of user_message_reference parameter

[Pablo Neira Ayuso]

Patch Set 2: Code-Review+1

Thanks a lot for fixing up this Pau.

-- 
To view, visit https://gerrit.osmocom.org/3554
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If748548a4a223e529a1110c89e483b599b406e8b
Gerrit-PatchSet: 2
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pablo Neira Ayuso 
Gerrit-HasComments: No

[PATCH] openbsc[master]: libmsc: Fix wrong handling of user_message_reference parameter

[Pau Espin Pedrol]

Hello Jenkins Builder,

I'd like you to reexamine a change.  Please visit

https://gerrit.osmocom.org/3554

to look at the new patch set (#2).

libmsc: Fix wrong handling of user_message_reference parameter

libsmpp34 already converts received TLV integer values to native
endianess in libsmpp34_(un)pack.

Converting them again at receive time swaps the 2 bytes of
user_message_reference, then using a wrong value. As GSM03.40 spec
uses only 1 byte for the id, then only the high byte of the initial
value is used and eventually sent back to the ESME. Again, at that time,
htons() is not needed because libsmpp34 already handles that part.

See OS-#2429 for more details.

Change-Id: If748548a4a223e529a1110c89e483b599b406e8b
---
M openbsc/src/libmsc/smpp_openbsc.c
1 file changed, 2 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/54/3554/2

diff --git a/openbsc/src/libmsc/smpp_openbsc.c 
b/openbsc/src/libmsc/smpp_openbsc.c
index e656376..af2d1be 100644
--- a/openbsc/src/libmsc/smpp_openbsc.c
+++ b/openbsc/src/libmsc/smpp_openbsc.c
@@ -120,7 +120,7 @@
}
break;
case TLVID_user_message_reference:
-   msg_ref = ntohs(t->value.val16);
+   msg_ref = t->value.val16;
break;
default:
break;
@@ -436,7 +436,7 @@
memset(, 0, sizeof(tlv));
tlv.tag = tag;
tlv.length = 2;
-   tlv.value.val16 = htons(val);
+   tlv.value.val16 = val;
build_tlv(req_tlv, );
 }
 

-- 
To view, visit https://gerrit.osmocom.org/3554
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If748548a4a223e529a1110c89e483b599b406e8b
Gerrit-PatchSet: 2
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pablo Neira Ayuso