September 2023 - gerrit-log - lists.osmocom.org

[S] Change in osmocom-bb[master]: modem: fix grr_st_packet_access_action(): shift rr->cr_hist properly

by fixeria

fixeria has submitted this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/34534?usp=email ) Change subject: modem: fix grr_st_packet_access_action(): shift rr->cr_hist properly ...................................................................... modem: fix grr_st_packet_access_action(): shift rr->cr_hist properly Pass size in bytes to memcpy(), not number of elements! Change-Id: I687435f5458e766d9d61143d6e4255f089fe1caf Fixes: 6db5f8b9c "modem: get rid of app_data.chan_req, use ms->rrlayer" Related: OS#5500, OS#6131 --- M src/host/layer23/src/modem/grr.c 1 file changed, 15 insertions(+), 1 deletion(-) Approvals: Jenkins Builder: Verified pespin: Looks good to me, approved diff --git a/src/host/layer23/src/modem/grr.c b/src/host/layer23/src/modem/grr.c index a029dd8..4e29909 100644 --- a/src/host/layer23/src/modem/grr.c +++ b/src/host/layer23/src/modem/grr.c @@ -688,7 +688,8 @@ } /* shift the CHANNEL REQUEST history buffer */ - memmove(&rr->cr_hist[1], &rr->cr_hist[0], ARRAY_SIZE(rr->cr_hist) - 1); + memmove(&rr->cr_hist[1], &rr->cr_hist[0], + sizeof(rr->cr_hist) - sizeof(rr->cr_hist[0])); /* store the new entry */ rr->cr_hist[0].ref = *ref; rr->cr_hist[0].ref.ra = rr->cr_ra; -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/34534?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: I687435f5458e766d9d61143d6e4255f089fe1caf Gerrit-Change-Number: 34534 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 year, 9 months

1
0
0 0

[L] Change in osmocom-bb[master]: ASCI: Add group receive and transmit mode support to MM layer

by fixeria

Attention is currently required from: jolly. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/34492?usp=email ) Change subject: ASCI: Add group receive and transmit mode support to MM layer ...................................................................... Patch Set 8: (3 comments) File src/host/layer23/src/mobile/gsm48_mm.c: https://gerrit.osmocom.org/c/osmocom-bb/+/34492/comment/10ea07b5_7619540d PS8, Line 4643: if (rr_prim != -1) { Doing `goto` from the `default` branch above might be a more elegant solution. (Gerrit highlighting is rather confusing, it looks as if the whole block was changed, but actually it was only shifted right). https://gerrit.osmocom.org/c/osmocom-bb/+/34492/comment/53ededef_24de43e8 PS8, Line 4813: nothing here TODO/FIXME? https://gerrit.osmocom.org/c/osmocom-bb/+/34492/comment/82842821_cdcae8a9 PS8, Line 4851: nothing here TODO/FIXME? -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/34492?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: I05957182a57423ad947ab200b52f65fde859e110 Gerrit-Change-Number: 34492 Gerrit-PatchSet: 8 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: jolly <andreas(a)eversberg.eu> Gerrit-Comment-Date: Tue, 26 Sep 2023 15:39:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 year, 9 months

1
0
0 0

[XL] Change in osmocom-bb[master]: ASCI: Add group receive mode support to RR layer

by fixeria

Attention is currently required from: jolly. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/34531?usp=email ) Change subject: ASCI: Add group receive mode support to RR layer ...................................................................... Patch Set 3: Code-Review+1 (1 comment) File src/host/layer23/src/mobile/gsm48_rr.c: https://gerrit.osmocom.org/c/osmocom-bb/+/34531/comment/df87faa1_090abc16 PS3, Line 5335: access bursts `an access burst`? -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/34531?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: Ia7d806b354fb3be5729bff8ac9aa1c7ad7a8b539 Gerrit-Change-Number: 34531 Gerrit-PatchSet: 3 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: jolly <andreas(a)eversberg.eu> Gerrit-Comment-Date: Tue, 26 Sep 2023 15:31:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 year, 9 months

1
0
0 0

[L] Change in osmocom-bb[master]: ASCI: Add ASCI notification support to RR layer

by fixeria

Attention is currently required from: jolly. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/34530?usp=email ) Change subject: ASCI: Add ASCI notification support to RR layer ...................................................................... Patch Set 3: Code-Review-1 (10 comments) File src/host/layer23/src/mobile/gsm48_rr.c: https://gerrit.osmocom.org/c/osmocom-bb/+/34530/comment/7ad01536_86ad894e PS3, Line 1331: uint8_t *gcr `const` https://gerrit.osmocom.org/c/osmocom-bb/+/34530/comment/56f2f0ee_e66ae20c PS3, Line 1387: uint8_t *gcr `const` https://gerrit.osmocom.org/c/osmocom-bb/+/34530/comment/7348a4ae_c9178028 PS3, Line 1399: uint8_t *gcr, uint8_t *ch_desc both `const` https://gerrit.osmocom.org/c/osmocom-bb/+/34530/comment/1bf90897_7b7b4b41 PS3, Line 1440: return -ENOMEM; You may be leaking the memory allocated by `asci_notif_alloc()` here. Maybe just use `OSMO_ASSERT(nmsg != NULL)`? NULL is unlikely anyway... https://gerrit.osmocom.org/c/osmocom-bb/+/34530/comment/fc1eebf2_4e6f1fe2 PS3, Line 1461: 5*8 The comment states 36 bits, but you allocate (5 * 8) bytes? I guess you need 5 bytes here, or just `OSMO_BYTES_FOR_BITS(36)`. https://gerrit.osmocom.org/c/osmocom-bb/+/34530/comment/37c943f4_10f54735 PS3, Line 1467: 3*8 Same here, `OSMO_BYTES_FOR_BITS(24)`, because: ``` \param[in] size Number of bytes in the vector ``` https://gerrit.osmocom.org/c/osmocom-bb/+/34530/comment/f2581c45_059f561f PS3, Line 1473: \n This is going to break the logstring formatting, better use just one `\n`. https://gerrit.osmocom.org/c/osmocom-bb/+/34530/comment/87f6829b_24176fad PS3, Line 1482: if (chd_bv) `bitvec_free()` is NULL safe, so these checks are redundant. https://gerrit.osmocom.org/c/osmocom-bb/+/34530/comment/596ddc34_3dea3de9 PS3, Line 1494: msgb_l3len(msg) - sizeof(*sgh); Is it guaranteed by the caller that `msgb_l3len(msg)` is always `>= sizeof(*sgh)`? If not, we may end up with a negative value here and thus huge bitvec length. AFAICS, it's not (see my comments to 34529). https://gerrit.osmocom.org/c/osmocom-bb/+/34530/comment/5e22edaa_c1637cdf PS3, Line 1519: msgb_l3len(msg) - sizeof(*nn); Same here. -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/34530?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: I49df02cb4d99d9aab1ea3ca13beb2ea00ae4c9f4 Gerrit-Change-Number: 34530 Gerrit-PatchSet: 3 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: jolly <andreas(a)eversberg.eu> Gerrit-Comment-Date: Tue, 26 Sep 2023 15:18:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 year, 9 months

1
0
0 0

[S] Change in osmocom-bb[master]: modem: fix grr_st_packet_access_action(): shift rr->cr_hist properly

by pespin

Attention is currently required from: fixeria. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/34534?usp=email ) Change subject: modem: fix grr_st_packet_access_action(): shift rr->cr_hist properly ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/34534?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: I687435f5458e766d9d61143d6e4255f089fe1caf Gerrit-Change-Number: 34534 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: Tue, 26 Sep 2023 15:10:03 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 year, 9 months

1
0
0 0

[M] Change in osmocom-bb[master]: ASCI: Prepare gsm48_rr_rx_acch for voice group channel

by fixeria

Attention is currently required from: jolly. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/34529?usp=email ) Change subject: ASCI: Prepare gsm48_rr_rx_acch for voice group channel ...................................................................... Patch Set 3: (4 comments) File src/host/layer23/src/mobile/gsm48_rr.c: https://gerrit.osmocom.org/c/osmocom-bb/+/34529/comment/56bd95db_5345a46b PS3, Line 4882: N201_Bter_SACCH These defines can also be found is libosmocore.git. But I see that they're not in a header file, unfortunately. https://gerrit.osmocom.org/c/osmocom-bb/+/34529/comment/89ef37d5_0ec61e83 PS3, Line 4889: struct gsm48_system_information_type_header *sih; : struct gsm48_hdr_sh *sgh; : struct gsm48_hdr *gh; all `const`? https://gerrit.osmocom.org/c/osmocom-bb/+/34529/comment/a4121561_73e6e496 PS3, Line 4895: sgh = msgb_l3(msg); do we want to check `if (msgb_l3len(msg) < sizeof(*sgh))` here? https://gerrit.osmocom.org/c/osmocom-bb/+/34529/comment/d194695f_932eac3a PS3, Line 4909: sih = msgb_l3(msg); do we want to check `if (msgb_l3len(msg) < sizeof(*sih))` here? -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/34529?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: I39b27396a31137b3c4bdcb40dccdf3de60458fe2 Gerrit-Change-Number: 34529 Gerrit-PatchSet: 3 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: jolly <andreas(a)eversberg.eu> Gerrit-Comment-Date: Tue, 26 Sep 2023 14:50:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 year, 9 months

1
0
0 0

[S] Change in osmocom-bb[master]: ASCI: Add interface for group receive/transmit mode support to RR layer

by fixeria

Attention is currently required from: jolly, pespin. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/34528?usp=email ) Change subject: ASCI: Add interface for group receive/transmit mode support to RR layer ...................................................................... Patch Set 3: Code-Review+2 (1 comment) Patchset: PS3: > As usually, I'd really like to get these introduced in the same patch where they are used. Do you want to spend hours reviewing one bomb patch? It's easier to review stuff when it's split across multiple commits, and looks like this was exactly the goal here. -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/34528?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: I1abd56c63d15af1cff8bde7589a571cb5bb5506f Gerrit-Change-Number: 34528 Gerrit-PatchSet: 3 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: jolly <andreas(a)eversberg.eu> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Tue, 26 Sep 2023 14:41:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 year, 9 months

1
0
0 0

[S] Change in osmocom-bb[master]: ASCI: Add channel notification event to MM events

by fixeria

Attention is currently required from: jolly. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/34488?usp=email ) Change subject: ASCI: Add channel notification event to MM events ...................................................................... Patch Set 8: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/34488?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: Ifee286ba4628356cc19b5dc75f1843287c5d2342 Gerrit-Change-Number: 34488 Gerrit-PatchSet: 8 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: jolly <andreas(a)eversberg.eu> Gerrit-Comment-Date: Tue, 26 Sep 2023 14:36:55 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 year, 9 months

1
0
0 0

[S] Change in osmocom-bb[master]: Fix request reference value in gsm48_match_ra()

by fixeria

Attention is currently required from: jolly, pespin. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/34486?usp=email ) Change subject: Fix request reference value in gsm48_match_ra() ...................................................................... Patch Set 8: (1 comment) File src/host/layer23/src/mobile/gsm48_rr.c: https://gerrit.osmocom.org/c/osmocom-bb/+/34486/comment/11748ca5_cfde56b7 PS8, Line 2365: struct gsm48_req_ref *ref I believe we would not have such a bug if this pointer was `const`. -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/34486?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: Iec636d368e20030beac2861bff61b1a06e7b4821 Gerrit-Change-Number: 34486 Gerrit-PatchSet: 8 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: jolly <andreas(a)eversberg.eu> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Tue, 26 Sep 2023 14:35:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 year, 9 months

1
0
0 0

[S] Change in osmocom-bb[master]: Fix request reference value in gsm48_match_ra()

by fixeria

Attention is currently required from: jolly, pespin. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/34486?usp=email ) Change subject: Fix request reference value in gsm48_match_ra() ...................................................................... Patch Set 8: Code-Review+2 (1 comment) File src/host/layer23/src/mobile/gsm48_rr.c: https://gerrit.osmocom.org/c/osmocom-bb/+/34486/comment/5223fa4c_d096097e PS5, Line 2377: ia_t1 = ref->t1; > There was a space in front of the third tab. Thanks for clarifying. I just found out that Gerrit does show this, but only when using the `Unified diff` instead of `Side-by-side diff`. -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/34486?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: Iec636d368e20030beac2861bff61b1a06e7b4821 Gerrit-Change-Number: 34486 Gerrit-PatchSet: 8 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: jolly <andreas(a)eversberg.eu> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Tue, 26 Sep 2023 14:30:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: jolly <andreas(a)eversberg.eu> Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 year, 9 months

1
0
0 0

2025

2024

2023

2022

gerrit-log September 2023