Change in osmo-sgsn[master]: gprs_gmm: send Service Reject when no PDP ctxs are available.

[Mykola Shchetinin]

Mykola Shchetinin has posted comments on this change. ( https://gerrit.osmocom.org/13744 )

Change subject: gprs_gmm: send Service Reject when no PDP ctxs are available.
......................................................................


Patch Set 4:

(5 comments)

https://gerrit.osmocom.org/#/c/13744/2/src/gprs/gprs_gmm.c
File src/gprs/gprs_gmm.c:

https://gerrit.osmocom.org/#/c/13744/2/src/gprs/gprs_gmm.c@1620
PS2, Line 1620: bool all_ms_ctx_present_on_sgsn(struct sgsn_mm_ctx *mmctx,
> You probably want to add spec chapter and section in this comment too.
Not sure about this place. The function does only perform a check. Well, do you mean the same section which mentioned in the change below? About "Service request procedure not accepted by the network". Yeah, that makes sense, as the function seems to be only related to that "part" of behavior


https://gerrit.osmocom.org/#/c/13744/2/src/gprs/gprs_gmm.c@1625
PS2, Line 1625: 	size_t i;
> where does this 16 come from? Can we please have a define (if we don't have already) for it?
Done


https://gerrit.osmocom.org/#/c/13744/2/src/gprs/gprs_gmm.c@1628
PS2, Line 1628: 		if (!(pdp_status[pdp_nsapi / 8] & (1 << (pdp_nsapi - 8 * i))))
> While fine at runtime, you are actually assigning an integer value (bitmask) to a bool here, that's  […]
Done


https://gerrit.osmocom.org/#/c/13744/2/src/gprs/gprs_gmm.c@1630
PS2, Line 1630: 
> Can we actually have something like: […]
Done


https://gerrit.osmocom.org/#/c/13744/2/src/gprs/gprs_gmm.c@1920
PS2, Line 1920: 	if (TLVP_PRESENT(&tp, GSM48_IE_GMM_PDP_CTX_STATUS)) {
> You probably need to drop this comment too right?
Hm, it seems fair to leave it as it correctly describes what is happening inside the condition. And it doesn't add redundancy to my comment below, does it?



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

Gerrit-Project: osmo-sgsn
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If610cbef17c25ec44e65d4f1b2340d102c560437
Gerrit-Change-Number: 13744
Gerrit-PatchSet: 4
Gerrit-Owner: Mykola Shchetinin <mykola at pentonet.com>
Gerrit-Reviewer: Jenkins Builder (1000002)
Gerrit-Reviewer: Mykola Shchetinin <mykola at pentonet.com>
Gerrit-Reviewer: lynxis lazus <lynxis at fe80.eu>
Gerrit-CC: Pau Espin Pedrol <pespin at sysmocom.de>
Gerrit-Comment-Date: Wed, 24 Apr 2019 11:34:27 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190424/0f988132/attachment.htm>