Attention is currently required from: fixeria.
neels has posted comments on this change. (
https://gerrit.osmocom.org/c/osmo-bsc/+/27509
)
Change subject: struct gsm_encr: store alg_id in human-readable format
......................................................................
Patch Set 2: Code-Review-1
(6 comments)
Commit Message:
https://gerrit.osmocom.org/c/osmo-bsc/+/27509/comment/bf470c69_00486e0f
PS2, Line 30: ciph_mod_set = (lchan->encr.alg_id << 1) | 1;
well, in Cipher Mode Setting, it has to be (a5_n - 1) << 1
File include/osmocom/bsc/gsm_data.h:
https://gerrit.osmocom.org/c/osmo-bsc/+/27509/comment/cea727ab_cb86df91
PS2, Line 613: uint8_t alg_id;
alg_id is named after the Algorithm Identifier IE.
I'd like to rename this member to 'a5_n' or 'alg_a5_n', because
- we use alg_id in various places, e.g. osmo-msc uses it as N+1, and it also exists in
libosmocore
- to me 'a5_n' most clearly indicates which representation it is storing.
- with a rename the patch shows every place in the code that uses this and we can easily
verify that all of them are correct.
File src/osmo-bsc/abis_rsl.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/27509/comment/5e15b741_e994f735
PS2, Line 163: switch (*out++) {
nitpick (ok, it's correct, but IMHO the code becomes far less head scratchy when we
don't ++ inline in conditionals. i'd have put a local variable there / maybe do
the switch on the human readable a5_n)
https://gerrit.osmocom.org/c/osmo-bsc/+/27509/comment/542605d6_0f78c2f8
PS2, Line 663: if (lchan->encr.alg_id > 0) {
(here is a place where i have to think twice, I thought it is incorrect at first; if the
name were a5_n, it would have helped me here)
File src/osmo-bsc/gsm_04_08_rr.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/27509/comment/0617ceb0_a94f7a3d
PS2, Line 596: _id
"- 1" is missing now! 44.018 10.5.2.9
File src/osmo-bsc/gsm_08_08.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/27509/comment/43598214_90ded639
PS2, Line 88: chosen_encr
also rename this to chosen_a5_n?
--
To view, visit
https://gerrit.osmocom.org/c/osmo-bsc/+/27509
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: Ieb50c9a352cfa5481aebac2379e0a461663543ec
Gerrit-Change-Number: 27509
Gerrit-PatchSet: 2
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 14 Mar 2022 11:40:55 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment