Pau Espin Pedrol has posted comments on this change. ( 
https://gerrit.osmocom.org/12969 )

Change subject: gprs_ns: Add missing NSVCI IE to NS-BLOCK-ACK message
......................................................................


Patch Set 1: Code-Review+1

(1 comment)

https://gerrit.osmocom.org/#/c/12969/1/src/gb/gprs_ns.c
File src/gb/gprs_ns.c:

https://gerrit.osmocom.org/#/c/12969/1/src/gb/gprs_ns.c@560
PS1, Line 560:  log_set_context(LOG_CTX_GB_NSVC, nsvc);
> the style of this function follows the style of all the other functions in 
> this file. […]
Fine, but then when you go back to select() and fire another path which 
contains no log_set_context(), you are still logging in that NSVC logging 
context while it may not be related at all. I have seen this kind of issue in 
osmo-bts (iirc) regarding IMSI logging context, where it was enabled but later 
not disabled, and as a result you start getting lots of non-related logging 
when filtering over IMSI.

But if all other functions in here do the same, let's keep it this way and we 
may want to check that later.



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

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7205e99d57f1e42d941f1be2460d8c9f46aadfe
Gerrit-Change-Number: 12969
Gerrit-PatchSet: 1
Gerrit-Owner: Harald Welte <[email protected]>
Gerrit-Reviewer: Harald Welte <[email protected]>
Gerrit-Reviewer: Jenkins Builder (1000002)
Gerrit-Reviewer: Max <[email protected]>
Gerrit-Reviewer: Pau Espin Pedrol <[email protected]>
Gerrit-Comment-Date: Wed, 20 Feb 2019 16:11:22 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: Yes

Reply via email to