January 2024 - gerrit-log - lists.osmocom.org

[L] Change in osmocom-bb[master]: mobile: init/deinit GAPK I/O based on CC transaction events
by pespin 17 Jan '24

17 Jan '24

Attention is currently required from: fixeria. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/35579?usp=email ) Change subject: mobile: init/deinit GAPK I/O based on CC transaction events ...................................................................... Patch Set 3: (1 comment) File src/host/layer23/include/osmocom/bb/mobile/tch.h: https://gerrit.osmocom.org/c/osmocom-bb/+/35579/comment/2b142e8c_2dce6080 PS3, Line 8: bool voice_or_data; /* voice (true) or data (false) */ > "totally misleading" sounds totally subjective to me :P But ok. it is totally misleading because when reading code "if (voice_or_data)" it clearly seems to (wrongly) indicate that it's either voice or data, not voice. -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/35579?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: I28f87f6a6de673611aa02a24e8985aee23d4498b Gerrit-Change-Number: 35579 Gerrit-PatchSet: 3 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Wed, 17 Jan 2024 10:51:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[M] Change in osmocom-bb[master]: mobile: add generic signals for CC/SS/SM transactions
by fixeria 17 Jan '24

17 Jan '24

Attention is currently required from: laforge, pespin. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/35577?usp=email ) Change subject: mobile: add generic signals for CC/SS/SM transactions ...................................................................... Patch Set 2: (1 comment) File src/host/layer23/include/osmocom/bb/common/osmocom_data.h: https://gerrit.osmocom.org/c/osmocom-bb/+/35577/comment/91ba4fdf_8366d7f5 PS2, Line 45: S_L23_CC_TRANS_ALLOC, /* new transaction has been allocated */ > maybe TRANS_NEW instead of alloc would be better, the fact of allocating is a bit too detailed imho. […] The current naming does reflect the lifetime of a transaction closely enough. Naming is always hard. IMO, not worth spending time on it, sorry. -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/35577?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: Idae5da24cb517878a26cc14b2ba6976e60f0b31b Gerrit-Change-Number: 35577 Gerrit-PatchSet: 2 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Wed, 17 Jan 2024 10:51:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[L] Change in osmocom-bb[master]: mobile: init/deinit GAPK I/O based on CC transaction events
by fixeria 17 Jan '24

17 Jan '24

Attention is currently required from: pespin. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/35579?usp=email ) Change subject: mobile: init/deinit GAPK I/O based on CC transaction events ...................................................................... Patch Set 3: (2 comments) Patchset: PS3: > I still have the feeling you are attempting too many changes in this single commit, but ok. Those are all related changes. I would prefer to spend more time fixing bugs and implementing the missing features, rather than splitting every second patch here onto 10 smaller ones with "this is a preparation patch for ...." or "this will be used by upcoming patch". File src/host/layer23/include/osmocom/bb/mobile/tch.h: https://gerrit.osmocom.org/c/osmocom-bb/+/35579/comment/35316314_212b420e PS3, Line 8: bool voice_or_data; /* voice (true) or data (false) */ > please name it "voice" or "is_voice". […] "totally misleading" sounds totally subjective to me :P But ok. -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/35579?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: I28f87f6a6de673611aa02a24e8985aee23d4498b Gerrit-Change-Number: 35579 Gerrit-PatchSet: 3 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Wed, 17 Jan 2024 10:48:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[S] Change in osmocom-bb[master]: mobile: gsm48_mm_data_ind(): check if struct gsm48_hdr fits
by pespin 17 Jan '24

17 Jan '24

Attention is currently required from: fixeria. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/35601?usp=email ) Change subject: mobile: gsm48_mm_data_ind(): check if struct gsm48_hdr fits ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/35601?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: Ibc5153df41e2c6365a3c65b1906d440a1074514b Gerrit-Change-Number: 35601 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Wed, 17 Jan 2024 10:40:16 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in osmocom-bb[master]: fixup: mobile: always check return value of tlv_parse()
by pespin 17 Jan '24

17 Jan '24

Attention is currently required from: fixeria. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/35602?usp=email ) Change subject: fixup: mobile: always check return value of tlv_parse() ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/35602?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: I51e2ae1be1c51a6359f8b0faad56f654251f1413 Gerrit-Change-Number: 35602 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Wed, 17 Jan 2024 10:40:12 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in osmocom-bb[master]: mobile: gsm48_mm_data_ind(): check if struct gsm48_hdr fits
by pespin 17 Jan '24

17 Jan '24

Attention is currently required from: fixeria. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/35601?usp=email ) Change subject: mobile: gsm48_mm_data_ind(): check if struct gsm48_hdr fits ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/35601?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: Ibc5153df41e2c6365a3c65b1906d440a1074514b Gerrit-Change-Number: 35601 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Wed, 17 Jan 2024 10:39:49 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in osmocom-bb[master]: mobile: add generic signals for CC/SS/SM transactions
by pespin 17 Jan '24

17 Jan '24

Attention is currently required from: fixeria. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/35577?usp=email ) Change subject: mobile: add generic signals for CC/SS/SM transactions ...................................................................... Patch Set 2: (1 comment) File src/host/layer23/include/osmocom/bb/common/osmocom_data.h: https://gerrit.osmocom.org/c/osmocom-bb/+/35577/comment/d20f46c1_0deb9016 PS2, Line 45: S_L23_CC_TRANS_ALLOC, /* new transaction has been allocated */ maybe TRANS_NEW instead of alloc would be better, the fact of allocating is a bit too detailed imho. This match more somehow what eg. netlink socket messages submit. Similarly same applies to TRANS_FREE. -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/35577?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: Idae5da24cb517878a26cc14b2ba6976e60f0b31b Gerrit-Change-Number: 35577 Gerrit-PatchSet: 2 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Wed, 17 Jan 2024 10:38:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[L] Change in osmocom-bb[master]: mobile: init/deinit GAPK I/O based on CC transaction events
by pespin 17 Jan '24

17 Jan '24

Attention is currently required from: fixeria. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/35579?usp=email ) Change subject: mobile: init/deinit GAPK I/O based on CC transaction events ...................................................................... Patch Set 3: (2 comments) Patchset: PS3: I still have the feeling you are attempting too many changes in this single commit, but ok. File src/host/layer23/include/osmocom/bb/mobile/tch.h: https://gerrit.osmocom.org/c/osmocom-bb/+/35579/comment/321e9776_14efbddc PS3, Line 8: bool voice_or_data; /* voice (true) or data (false) */ please name it "voice" or "is_voice". Otherwise code like "if (voice_or_data)" is totally misleading. -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/35579?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: I28f87f6a6de673611aa02a24e8985aee23d4498b Gerrit-Change-Number: 35579 Gerrit-PatchSet: 3 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Wed, 17 Jan 2024 10:37:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in osmocom-bb[master]: mobile: cosmetic: use consistent naming for gapk_io_state
by pespin 17 Jan '24

17 Jan '24

Attention is currently required from: fixeria. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/35600?usp=email ) Change subject: mobile: cosmetic: use consistent naming for gapk_io_state ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/35600?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: I96a995bdf20c95a15271de68fbf434e101047313 Gerrit-Change-Number: 35600 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Wed, 17 Jan 2024 10:33:14 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in osmocom-bb[master]: fixup: mobile: always check return value of tlv_parse()
by fixeria 16 Jan '24

16 Jan '24

fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmocom-bb/+/35602?usp=email ) Change subject: fixup: mobile: always check return value of tlv_parse() ...................................................................... fixup: mobile: always check return value of tlv_parse() Fix a regression: check if Location Area Information IE fits. Change-Id: I51e2ae1be1c51a6359f8b0faad56f654251f1413 Fixes: bb0ac02e "mobile: always check return value of tlv_parse()" Fixes: CID#341618 --- M src/host/layer23/src/mobile/gsm48_mm.c 1 file changed, 14 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/osmocom-bb refs/changes/02/35602/1 diff --git a/src/host/layer23/src/mobile/gsm48_mm.c b/src/host/layer23/src/mobile/gsm48_mm.c index ee457ad..fad475e 100644 --- a/src/host/layer23/src/mobile/gsm48_mm.c +++ b/src/host/layer23/src/mobile/gsm48_mm.c @@ -2613,7 +2613,7 @@ struct tlv_parsed tp; struct msgb *nmsg; - if (payload_len < 0) { + if (payload_len < sizeof(*lai)) { short_read: LOGP(DMM, LOGL_ERROR, "Short read of LOCATION UPDATING ACCEPT message\n"); return -EINVAL; -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/35602?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: I51e2ae1be1c51a6359f8b0faad56f654251f1413 Gerrit-Change-Number: 35602 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: newchange

1 0