fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/33411 )
Change subject: coding: fix a copy-paste bug in gsm0503_tch_afs_decode_dtx()
......................................................................
coding: fix a copy-paste bug in gsm0503_tch_afs_decode_dtx()
According to 3GPP TS 45.003, section 3.9.2, the convolutional coding
is *not* employed in the case of SID_FIRST. This is why we're using
sid_first_dummy[] in this function.
The conv[] array is not initialized in the case of AFS_SID_FIRST and
passing it to tch_amr_reassemble() is highly likely a copy-paste bug.
Pass the sid_first_dummy[] instead.
Change-Id: Ic8bfc34ce9d14821fe4e82932325f0c517d443a1
Fixes: CID#272949 "Uninitialized scalar variable"
---
M src/coding/gsm0503_coding.c
1 file changed, 19 insertions(+), 1 deletion(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/11/33411/1
diff --git a/src/coding/gsm0503_coding.c b/src/coding/gsm0503_coding.c
index 80286f7..286981e 100644
--- a/src/coding/gsm0503_coding.c
+++ b/src/coding/gsm0503_coding.c
@@ -2262,7 +2262,7 @@
tch_amr_sid_update_append(sid_first_dummy, 0,
(codec_mode_req) ? codec[*ft]
: codec[id > 0 ? id : 0]);
- tch_amr_reassemble(tch_data, conv, 39);
+ tch_amr_reassemble(tch_data, sid_first_dummy, 39);
len = 5;
goto out;
case AFS_SID_UPDATE: /* TODO: parse CMI _and_ CMC/CMR (16 + 16 bit) */
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/33411
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Ic8bfc34ce9d14821fe4e82932325f0c517d443a1
Gerrit-Change-Number: 33411
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: newchange
Attention is currently required from: osmith, fixeria.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/33400 )
Change subject: Fix various uninitialized value errors
......................................................................
Patch Set 1: Code-Review-1
(5 comments)
File src/coding/gsm0503_coding.c:
https://gerrit.osmocom.org/c/libosmocore/+/33400/comment/fc61231a_3dbc778f
PS1, Line 2197: ubit_t d[244], p[6], conv[250] = {};
This definetly needs an explanation in commit description and possibly its own commit.
File src/gsm/gsm23236.c:
https://gerrit.osmocom.org/c/libosmocore/+/33400/comment/d659e1c3_e0fe40ec
PS1, Line 490: struct osmo_nri_range add_range = {};
I don't really see a problem with not initializing it, unless osmo_nri_parse_range() requires the first param to be 0 initialized. Possibly something to be clarified.
In any case, this requires explanation in commit description and its own commit.
https://gerrit.osmocom.org/c/libosmocore/+/33400/comment/d9bdd82d_92569161
PS1, Line 496: if (added_range)
imho this should be moved after the if in line 499.
https://gerrit.osmocom.org/c/libosmocore/+/33400/comment/27ea52c8_18be4aca
PS1, Line 526: struct osmo_nri_range del_range = {};
same as add_range above.
File tests/codec/codec_ecu_fr_test.c:
https://gerrit.osmocom.org/c/libosmocore/+/33400/comment/3c51d659_b8d92561
PS1, Line 126: osmo_ecu_fr_reset(&state, frame);
why is zero-initializing state above required when calling this function? does osmo_ecu_fr_reset() require it? is it left uninitialized if osmo_ecu_fr_reset() fails?
Further expalanation needed + own commit.
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/33400
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Icaf53fda8752e9d2bd2cfe8c7b89bb31b90f684f
Gerrit-Change-Number: 33400
Gerrit-PatchSet: 1
Gerrit-Owner: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 23 Jun 2023 09:51:30 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: osmith, fixeria, pespin.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/33400 )
Change subject: Fix various uninitialized value errors
......................................................................
Patch Set 1: Code-Review-1
(1 comment)
File src/coding/gsm0503_coding.c:
https://gerrit.osmocom.org/c/libosmocore/+/33400/comment/027f7cb1_42c52499
PS1, Line 2197: conv[250] = {};
I don't think we should be doing zero-init unconditionally here. This is a hot code path, and stuff like that should be avoided when possible. Let me take a look at CID#272949.
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/33400
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Icaf53fda8752e9d2bd2cfe8c7b89bb31b90f684f
Gerrit-Change-Number: 33400
Gerrit-PatchSet: 1
Gerrit-Owner: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 23 Jun 2023 09:47:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment