openbsc[master]: gsm04_08_clear_request(): release loc with arg release=0

Submitted by on May 21, 2016, 2:44 p.m.


Message ID
State New
Series "openbsc[master]: gsm04_08_clear_request(): release loc with arg release=0"
Headers show

Commit Message May 21, 2016, 2:44 p.m.
Review at

gsm04_08_clear_request(): release loc with arg release=0

In gsm04_08_clear_request(), in_release == 1 anyway and
msc_release_connection() would exit immediately without any effect. Don't
confuse the reader by passing release=1 arg.

Change-Id: I5bf9eb4889d32ad5e42ac7d096bf62fa3a493e20
M openbsc/src/libmsc/gsm_04_08.c
1 file changed, 1 insertion(+), 1 deletion(-)

  git pull ssh:// refs/changes/93/93/1

Patch hide | download patch | download mbox

diff --git a/openbsc/src/libmsc/gsm_04_08.c b/openbsc/src/libmsc/gsm_04_08.c
index f02f784..f1b95c1 100644
--- a/openbsc/src/libmsc/gsm_04_08.c
+++ b/openbsc/src/libmsc/gsm_04_08.c
@@ -384,7 +384,7 @@ 
 	 * Cancel any outstanding location updating request
 	 * operation taking place on the subscriber connection.
-	release_loc_updating_req(conn, 1);
+	release_loc_updating_req(conn, 0);
 	/* We might need to cancel the paging response or such. */
 	if (conn->sec_operation && conn->sec_operation->cb) {

Comments May 22, 2016, 11:16 a.m.
Patch Set 2:

I think what we can argue about is. Either way you need to wonder how the calls deal in case of the release.

* Either you know that release_loc_updating_req can be asked not to msc_release_connection
* Or you know that because you set in_relase = 1, msc_release_connection will be called but that it is a no-op.

So do you really think that doing it this way is more readable/more obvious? May 22, 2016, 11:54 a.m.
Patch Set 2:

I know that in_release==1 so msc_release_connection() is a nop.
It is really weird to set a flag that disables a function completely
and then call that function anyway.

I find that confusing, but if you don't agree we can drop this change. May 23, 2016, 4:25 p.m.
Patch Set 2: Code-Review+2

I have no preference here. I just think we exchange one requirement of knowledge about the code with another one.