Change in osmo-bsc[master]: cosmetic: logging and ordering in handle_ass_compl()

[Harald Welte]

Harald Welte has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/9230 )

Change subject: cosmetic: logging and ordering in handle_ass_compl()
..

cosmetic: logging and ordering in handle_ass_compl()

* Check the message length once at the start, before any other actions.
* Use only one local gsm48_hdr pointer.
* Read the cause value once near the top, re-use it.
* Log "ASSIGNMENT COMPLETE" always, not only during handover.
* Fully initialize local struct lchan_signal_data.

Change-Id: Idcfd932d3dfb0b621ed6d8c4f92c0231abcdcec8
---
M src/libbsc/bsc_api.c
1 file changed, 21 insertions(+), 19 deletions(-)

Approvals:
  Jenkins Builder: Verified
  Harald Welte: Looks good to me, approved



diff --git a/src/libbsc/bsc_api.c b/src/libbsc/bsc_api.c
index 71cc0f0..8ae781e 100644
--- a/src/libbsc/bsc_api.c
+++ b/src/libbsc/bsc_api.c
@@ -344,18 +344,27 @@
 static void handle_ass_compl(struct gsm_subscriber_connection *conn,
 struct msgb *msg)
 {
-   struct gsm48_hdr *gh;
+   struct gsm48_hdr *gh = msgb_l3(msg);
struct bsc_api *api = conn->network->bsc_api;
+   enum gsm48_rr_cause cause;
+
+   /* Expecting gsm48_hdr + cause value */
+   if (msgb_l3len(msg) != sizeof(*gh) + 1) {
+   LOGPLCHAN(msg->lchan, DRR, LOGL_ERROR,
+ "RR Assignment Complete: length invalid: %u, expected 
%zu\n",
+ msgb_l3len(msg), sizeof(*gh) + 1);
+   return;
+   }
+
+   cause = gh->data[0];
+
+   LOGPLCHAN(msg->lchan, DRR, LOGL_DEBUG, "ASSIGNMENT COMPLETE cause = 
%s\n",
+ rr_cause_name(cause));

if (conn->ho) {
-   struct lchan_signal_data sig;
-   struct gsm48_hdr *gh = msgb_l3(msg);
-
-   LOGPLCHAN(msg->lchan, DRR, LOGL_DEBUG, "ASSIGNMENT COMPLETE 
cause = %s\n",
- rr_cause_name(gh->data[0]));
-
-   sig.lchan = msg->lchan;
-   sig.mr = NULL;
+   struct lchan_signal_data sig = {
+   .lchan = msg->lchan,
+   };
osmo_signal_dispatch(SS_LCHAN, S_LCHAN_ASSIGNMENT_COMPL, );
/* FIXME: release old channel */

@@ -366,15 +375,8 @@
}

if (conn->secondary_lchan != msg->lchan) {
-   LOGPLCHAN(msg->lchan, DMSC, LOGL_ERROR,
- "Assignment Compl should occur on second lchan.\n");
-   return;
-   }
-
-   gh = msgb_l3(msg);
-   if (msgb_l3len(msg) - sizeof(*gh) != 1) {
-   LOGPLCHAN(msg->lchan, DMSC, LOGL_ERROR, "Assignment Compl 
invalid: %zu\n",
- msgb_l3len(msg) - sizeof(*gh));
+   LOGPLCHAN(msg->lchan, DRR, LOGL_ERROR,
+ "RR Assignment Complete does not match conn's 
secondary lchan.\n");
return;
}

@@ -391,7 +393,7 @@
if (is_ipaccess_bts(conn_get_bts(conn)) && conn->lchan->tch_mode != 
GSM48_CMODE_SIGN)
rsl_ipacc_crcx(conn->lchan);

-   api->assign_compl(conn, gh->data[0]);
+   api->assign_compl(conn, cause);
 }

 static void handle_ass_fail(struct gsm_subscriber_connection *conn,

--
To view, visit https://gerrit.osmocom.org/9230
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Idcfd932d3dfb0b621ed6d8c4f92c0231abcdcec8
Gerrit-Change-Number: 9230
Gerrit-PatchSet: 3
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder

Change in osmo-bsc[master]: cosmetic: logging and ordering in handle_ass_compl()

[Harald Welte]

Harald Welte has posted comments on this change. ( 
https://gerrit.osmocom.org/9230 )

Change subject: cosmetic: logging and ordering in handle_ass_compl()
..


Patch Set 3: Code-Review+2


--
To view, visit https://gerrit.osmocom.org/9230
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idcfd932d3dfb0b621ed6d8c4f92c0231abcdcec8
Gerrit-Change-Number: 9230
Gerrit-PatchSet: 3
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Comment-Date: Wed, 23 May 2018 16:02:06 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes

Change in osmo-bsc[master]: cosmetic: logging and ordering in handle_ass_compl()

[Neels Hofmeyr]

Hello Jenkins Builder,

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

https://gerrit.osmocom.org/9230

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

Change subject: cosmetic: logging and ordering in handle_ass_compl()
..

cosmetic: logging and ordering in handle_ass_compl()

* Check the message length once at the start, before any other actions.
* Use only one local gsm48_hdr pointer.
* Read the cause value once near the top, re-use it.
* Log "ASSIGNMENT COMPLETE" always, not only during handover.
* Fully initialize local struct lchan_signal_data.

Change-Id: Idcfd932d3dfb0b621ed6d8c4f92c0231abcdcec8
---
M src/libbsc/bsc_api.c
1 file changed, 21 insertions(+), 19 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/30/9230/2
--
To view, visit https://gerrit.osmocom.org/9230
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Idcfd932d3dfb0b621ed6d8c4f92c0231abcdcec8
Gerrit-Change-Number: 9230
Gerrit-PatchSet: 2
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Jenkins Builder

Change in osmo-bsc[master]: cosmetic: logging and ordering in handle_ass_compl()

[Neels Hofmeyr]

Neels Hofmeyr has uploaded this change for review. ( 
https://gerrit.osmocom.org/9230


Change subject: cosmetic: logging and ordering in handle_ass_compl()
..

cosmetic: logging and ordering in handle_ass_compl()

* Check the message length once at the start, before any other actions.
* Use only one local gsm48_hdr pointer.
* Read the cause value once near the top, re-use it.
* Log "ASSIGNMENT COMPLETE" always, not only during handover.
* Fully initialize local struct lchan_signal_data.

Change-Id: Idcfd932d3dfb0b621ed6d8c4f92c0231abcdcec8
---
M src/libbsc/bsc_api.c
1 file changed, 21 insertions(+), 19 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/30/9230/1

diff --git a/src/libbsc/bsc_api.c b/src/libbsc/bsc_api.c
index 71cc0f0..c6c1438 100644
--- a/src/libbsc/bsc_api.c
+++ b/src/libbsc/bsc_api.c
@@ -344,18 +344,27 @@
 static void handle_ass_compl(struct gsm_subscriber_connection *conn,
 struct msgb *msg)
 {
-   struct gsm48_hdr *gh;
+   struct gsm48_hdr *gh = msgb_l3(msg);
struct bsc_api *api = conn->network->bsc_api;
+   enum gsm48_rr_cause cause;
+
+   /* Expecting gsm48_hdr + cause value */
+   if (msgb_l3len(msg) != sizeof(*gh) + 1) {
+   LOGPLCHAN(msg->lchan, DRR, LOGL_ERROR,
+ "RR Assignment Complete: length invalid: %zu, 
expected %zu\n",
+ msgb_l3len(msg), sizeof(*gh) + 1);
+   return;
+   }
+
+   cause = gh->data[0];
+
+   LOGPLCHAN(msg->lchan, DRR, LOGL_DEBUG, "ASSIGNMENT COMPLETE cause = 
%s\n",
+ rr_cause_name(cause));

if (conn->ho) {
-   struct lchan_signal_data sig;
-   struct gsm48_hdr *gh = msgb_l3(msg);
-
-   LOGPLCHAN(msg->lchan, DRR, LOGL_DEBUG, "ASSIGNMENT COMPLETE 
cause = %s\n",
- rr_cause_name(gh->data[0]));
-
-   sig.lchan = msg->lchan;
-   sig.mr = NULL;
+   struct lchan_signal_data sig = {
+   .lchan = msg->lchan,
+   };
osmo_signal_dispatch(SS_LCHAN, S_LCHAN_ASSIGNMENT_COMPL, );
/* FIXME: release old channel */

@@ -366,15 +375,8 @@
}

if (conn->secondary_lchan != msg->lchan) {
-   LOGPLCHAN(msg->lchan, DMSC, LOGL_ERROR,
- "Assignment Compl should occur on second lchan.\n");
-   return;
-   }
-
-   gh = msgb_l3(msg);
-   if (msgb_l3len(msg) - sizeof(*gh) != 1) {
-   LOGPLCHAN(msg->lchan, DMSC, LOGL_ERROR, "Assignment Compl 
invalid: %zu\n",
- msgb_l3len(msg) - sizeof(*gh));
+   LOGPLCHAN(msg->lchan, DRR, LOGL_ERROR,
+ "RR Assignment Complete does not match conn's 
secondary lchan.\n");
return;
}

@@ -391,7 +393,7 @@
if (is_ipaccess_bts(conn_get_bts(conn)) && conn->lchan->tch_mode != 
GSM48_CMODE_SIGN)
rsl_ipacc_crcx(conn->lchan);

-   api->assign_compl(conn, gh->data[0]);
+   api->assign_compl(conn, cause);
 }
 
 static void handle_ass_fail(struct gsm_subscriber_connection *conn,

--
To view, visit https://gerrit.osmocom.org/9230
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Idcfd932d3dfb0b621ed6d8c4f92c0231abcdcec8
Gerrit-Change-Number: 9230
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr