- gerrit-log - lists.osmocom.org

[S] Change in osmo-bsc[master]: abis_nm: fix -Wunused-but-set-variable (bug)
by fixeria 05 Feb '24

05 Feb '24

Attention is currently required from: laforge, pespin. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/35791?usp=email ) Change subject: abis_nm: fix -Wunused-but-set-variable (bug) ...................................................................... Patch Set 3: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/35791?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I3ca8e761fdf99dd498a979ccc9d53c6c3e03e2cc Gerrit-Change-Number: 35791 Gerrit-PatchSet: 3 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> 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: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Mon, 05 Feb 2024 13:48:25 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in osmo-bsc[master]: abis_nm_sw_act_req_ack: attr_len should be unsigned
by fixeria 05 Feb '24

05 Feb '24

Attention is currently required from: pespin. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/35856?usp=email ) Change subject: abis_nm_sw_act_req_ack: attr_len should be unsigned ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/35856?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I5c0ba76b5cc2464c7e362a850325c94770f38397 Gerrit-Change-Number: 35856 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-CC: Jenkins Builder Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Mon, 05 Feb 2024 13:48:04 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in osmo-bsc[master]: abis_nm: fix -Wunused-but-set-variable (bug)
by pespin 05 Feb '24

05 Feb '24

Attention is currently required from: fixeria, laforge. pespin has uploaded a new patch set (#3) to the change originally created by fixeria. ( https://gerrit.osmocom.org/c/osmo-bsc/+/35791?usp=email ) The following approvals got outdated and were removed: Code-Review+1 by laforge, Verified+1 by Jenkins Builder Change subject: abis_nm: fix -Wunused-but-set-variable (bug) ...................................................................... abis_nm: fix -Wunused-but-set-variable (bug) clang warns us about 'len' being set, but not used: And this is abis_nm.c:2172:10: warning: variable 'len' set but not used [-Wunused-but-set-variable] uint8_t len = attr_len; ^ This is actually a bug, because in the case of NACK we append 2 more bytes {NM_ATT_NACK_CAUSES, NM_NACK_OBJCLASS_NOTSUPP}, and we need to pass the final length to fill_om_fom_hdr(), including those optional two bytes. Passing 'attr_len' (length of 'attr') is wrong. Change-Id: I3ca8e761fdf99dd498a979ccc9d53c6c3e03e2cc --- M src/osmo-bsc/abis_nm.c 1 file changed, 21 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/91/35791/3 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/35791?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I3ca8e761fdf99dd498a979ccc9d53c6c3e03e2cc Gerrit-Change-Number: 35791 Gerrit-PatchSet: 3 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: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0

[S] Change in osmo-bsc[master]: abis_nm_sw_act_req_ack: attr_len should be unsigned
by pespin 05 Feb '24

05 Feb '24

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-bsc/+/35856?usp=email ) Change subject: abis_nm_sw_act_req_ack: attr_len should be unsigned ...................................................................... abis_nm_sw_act_req_ack: attr_len should be unsigned Change-Id: I5c0ba76b5cc2464c7e362a850325c94770f38397 --- M include/osmocom/bsc/abis_nm.h M src/osmo-bsc/abis_nm.c 2 files changed, 17 insertions(+), 3 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/56/35856/1 diff --git a/include/osmocom/bsc/abis_nm.h b/include/osmocom/bsc/abis_nm.h index 59a6dc9..f55c29c 100644 --- a/include/osmocom/bsc/abis_nm.h +++ b/include/osmocom/bsc/abis_nm.h @@ -87,7 +87,7 @@ int abis_nm_set_channel_attr(struct gsm_bts_trx_ts *ts, uint8_t chan_comb); int abis_nm_sw_act_req_ack(struct gsm_bts *bts, uint8_t obj_class, uint8_t i1, uint8_t i2, uint8_t i3, int nack, - const uint8_t *attr, int attr_len); + const uint8_t *attr, unsigned int attr_len); int abis_nm_raw_msg(struct gsm_bts *bts, int len, uint8_t *msg); int abis_nm_event_reports(struct gsm_bts *bts, int on); int abis_nm_reset_resource(struct gsm_bts *bts); diff --git a/src/osmo-bsc/abis_nm.c b/src/osmo-bsc/abis_nm.c index e8327dd..7a9789b 100644 --- a/src/osmo-bsc/abis_nm.c +++ b/src/osmo-bsc/abis_nm.c @@ -756,11 +756,16 @@ DEBUGPFOH(DNM, foh, "Software Activate Request, ACKing and Activating\n"); + if (oh->length < sizeof(*foh)) { + LOGPFOH(DNM, LOGL_ERROR, foh, "Software Activate Request with length too small: %u\n", oh->length); + return -EINVAL; + } + ret = abis_nm_sw_act_req_ack(sign_link->trx->bts, foh->obj_class, foh->obj_inst.bts_nr, foh->obj_inst.trx_nr, foh->obj_inst.ts_nr, 0, - foh->data, oh->length-sizeof(*foh)); + foh->data, oh->length - sizeof(*foh)); if (ret != 0) { LOGPFOH(DNM, LOGL_ERROR, foh, "Sending SW ActReq ACK failed: %d\n", ret); return ret; @@ -2165,7 +2170,7 @@ int abis_nm_sw_act_req_ack(struct gsm_bts *bts, uint8_t obj_class, uint8_t i1, uint8_t i2, uint8_t i3, int nack, - const uint8_t *attr, int attr_len) + const uint8_t *attr, unsigned int attr_len) { struct abis_om_hdr *oh; struct msgb *msg = nm_msgb_alloc(); -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/35856?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I5c0ba76b5cc2464c7e362a850325c94770f38397 Gerrit-Change-Number: 35856 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

1 0

[S] Change in osmo-bsc[master]: abis_nm: fix -Wunused-but-set-variable (bug)
by pespin 05 Feb '24

05 Feb '24

Attention is currently required from: fixeria. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/35791?usp=email ) Change subject: abis_nm: fix -Wunused-but-set-variable (bug) ...................................................................... Patch Set 2: (1 comment) File src/osmo-bsc/abis_nm.c: https://gerrit.osmocom.org/c/osmo-bsc/+/35791/comment/647e4fa0_552d4943 PS2, Line 2168: const uint8_t *attr, int attr_len) > Is this really important who does this check? Feel free to submit improvements and/or take over this […] Ok I'm fixing it now. -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/35791?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I3ca8e761fdf99dd498a979ccc9d53c6c3e03e2cc Gerrit-Change-Number: 35791 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: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Mon, 05 Feb 2024 13:36:13 +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

[S] Change in osmocom-bb[master]: trxcon/l1sched: trigger sending UL BLOCK.cnf for PDTCH
by fixeria 05 Feb '24

05 Feb '24

Attention is currently required from: jolly. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/35778?usp=email ) Change subject: trxcon/l1sched: trigger sending UL BLOCK.cnf for PDTCH ...................................................................... Patch Set 1: (2 comments) File src/host/trxcon/src/sched_lchan_pdtch.c: https://gerrit.osmocom.org/c/osmocom-bb/+/35778/comment/c5a6b507_55d351ff PS1, Line 164: lchan->prim = msg I will add an `ASSERT` to make sure `lchan->prim` is NULL here. https://gerrit.osmocom.org/c/osmocom-bb/+/35778/comment/4d8458da_dc9c81e3 PS1, Line 191: } > What happens to lchan->prim, if bid is not 3? `lchan->prim` should remain allocated and assigned until `bid == 3` (4th burst) - this is when we want to confirm sending UL BLOCK.req to the upper layers, because burst timing matters. -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/35778?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: If32fafeef0ea347ed3800e6b67349bf12e66047f Gerrit-Change-Number: 35778 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: jolly <andreas(a)eversberg.eu> Gerrit-Attention: jolly <andreas(a)eversberg.eu> Gerrit-Comment-Date: Mon, 05 Feb 2024 13:05:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: jolly <andreas(a)eversberg.eu> Gerrit-MessageType: comment

1 0

[S] Change in osmocom-bb[master]: trxcon/l1sched: trigger sending UL BLOCK.cnf for PDTCH
by jolly 05 Feb '24

05 Feb '24

Attention is currently required from: fixeria. jolly has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/35778?usp=email ) Change subject: trxcon/l1sched: trigger sending UL BLOCK.cnf for PDTCH ...................................................................... Patch Set 1: (2 comments) Patchset: PS1: > What happens to lchan->prim, if bid is not 3? Oops, wrong location for this comment. File src/host/trxcon/src/sched_lchan_pdtch.c: https://gerrit.osmocom.org/c/osmocom-bb/+/35778/comment/6d956dbd_1ecf9ec8 PS1, Line 191: } What happens to lchan->prim, if bid is not 3? -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/35778?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: If32fafeef0ea347ed3800e6b67349bf12e66047f Gerrit-Change-Number: 35778 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: jolly <andreas(a)eversberg.eu> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Mon, 05 Feb 2024 13:01:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: jolly <andreas(a)eversberg.eu> Gerrit-MessageType: comment

1 0

[S] Change in osmo-bsc[master]: abis_nm: fix -Wunused-but-set-variable (bug)
by fixeria 05 Feb '24

05 Feb '24

Attention is currently required from: pespin. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/35791?usp=email ) Change subject: abis_nm: fix -Wunused-but-set-variable (bug) ...................................................................... Patch Set 2: (1 comment) File src/osmo-bsc/abis_nm.c: https://gerrit.osmocom.org/c/osmo-bsc/+/35791/comment/01f49303_e8492658 PS2, Line 2168: const uint8_t *attr, int attr_len) > My point is that perhaps that check should be done by whoever does "oh->length - sizeof(*foh)", and […] Is this really important who does this check? Feel free to submit improvements and/or take over this patch. I really don't see a good reason to spend too much time here, sorry. All I wanted to do is fixing a compilation warning, so that I could build osmo-bsc with `CC=clang` and `-Werror=all`. And fixed an actual bug while doing so. -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/35791?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I3ca8e761fdf99dd498a979ccc9d53c6c3e03e2cc Gerrit-Change-Number: 35791 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: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Mon, 05 Feb 2024 13:01:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[S] Change in osmocom-bb[master]: trxcon/l1sched: trigger sending UL BLOCK.cnf for PDTCH
by jolly 05 Feb '24

05 Feb '24

Attention is currently required from: fixeria. jolly has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/35778?usp=email ) Change subject: trxcon/l1sched: trigger sending UL BLOCK.cnf for PDTCH ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: What happens to lchan->prim, if bid is not 3? -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/35778?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: If32fafeef0ea347ed3800e6b67349bf12e66047f Gerrit-Change-Number: 35778 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: jolly <andreas(a)eversberg.eu> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Mon, 05 Feb 2024 12:58:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in osmo-bsc[master]: abis_nm: fix -Wunused-but-set-variable (bug)
by pespin 05 Feb '24

05 Feb '24

Attention is currently required from: fixeria. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/35791?usp=email ) Change subject: abis_nm: fix -Wunused-but-set-variable (bug) ...................................................................... Patch Set 2: (1 comment) File src/osmo-bsc/abis_nm.c: https://gerrit.osmocom.org/c/osmo-bsc/+/35791/comment/4a8cd56d_d30731fc PS2, Line 2168: const uint8_t *attr, int attr_len) > This function gets `attr` / `attr_len` from an non-trusted source (received OML message). […] My point is that perhaps that check should be done by whoever does "oh->length - sizeof(*foh)", and not somehow delay checking that until inside this function. -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/35791?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I3ca8e761fdf99dd498a979ccc9d53c6c3e03e2cc Gerrit-Change-Number: 35791 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: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Mon, 05 Feb 2024 12:56:12 +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

2026

2025

2024

2023

2022

gerrit-log