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 <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels <[email protected]> Gerrit-Reviewer: pespin <[email protected]> Gerrit-Attention: fixeria <[email protected]> Gerrit-Comment-Date: Mon, 14 Mar 2022 11:40:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
