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

[S] Change in osmo-bts[master]: l1sap: fix wording in comment

by pespin

Attention is currently required from: dexter. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/32610 ) Change subject: l1sap: fix wording in comment ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32610 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: Ie7fcc53dea462b0d575b0c9ca73ba7507289eefe Gerrit-Change-Number: 32610 Gerrit-PatchSet: 1 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Thu, 04 May 2023 16:01:25 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 1 month

1
0
0 0

[M] Change in osmo-bts[master]: l1sap: Accept RFC5993 and TS 101.318 HR GSM payload

by pespin

Attention is currently required from: jolly, laforge, fixeria, dexter. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/31417 ) Change subject: l1sap: Accept RFC5993 and TS 101.318 HR GSM payload ...................................................................... Patch Set 8: (3 comments) File src/common/l1sap.c: https://gerrit.osmocom.org/c/osmo-bts/+/31417/comment/df6fc162_d2a603aa PS8, Line 1244: static bool rtppayload_is_valid(struct gsm_lchan *lchan, struct msgb *resp_msg) if this is expected to be used only for incoming DL packets we forward to lower layers towards the MS, we should really make this function name more descriptive. Because with current name it really looks like it could be used in any direction. https://gerrit.osmocom.org/c/osmo-bts/+/31417/comment/ba4ca3e6_53682eed PS8, Line 1274: /* BTS flags specify that RFC 5993 (and not ETSI TS 101.318) is understood by lower layers */ All this code block below you are writing can iiuc be simplified to: if (resp_msg->len == GSM_HR_BYTES_RTP_RFC5993) return rfc5993; if (resp_msg->len == GSM_HR_BYTES_RTP_TS101318) return ts101318; return false; https://gerrit.osmocom.org/c/osmo-bts/+/31417/comment/89f8f043_35819518 PS8, Line 1952: * formats are supported (either by setting both or none of the flags), no conversion will be carried out. */ we don't care about "both formats being supported". We care about the received format being supported. BTW, what's the point of checking the supported formats in rtppayload_is_valid() if we are anyway converting them here to whatever the supported format is? What am I missing? -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/31417 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I453562da412fde5b928bd2b588129c58ec8e2a7e Gerrit-Change-Number: 31417 Gerrit-PatchSet: 8 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: jolly <andreas(a)eversberg.eu> 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-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Thu, 04 May 2023 16:00:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

2 years, 1 month

1
0
0 0

[M] Change in osmo-bts[master]: l1sap: Accept RFC5993 and TS 101.318 HR GSM payload

by pespin

Attention is currently required from: jolly, laforge, fixeria, dexter. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/31417 ) Change subject: l1sap: Accept RFC5993 and TS 101.318 HR GSM payload ...................................................................... Patch Set 8: (1 comment) File src/common/l1sap.c: https://gerrit.osmocom.org/c/osmo-bts/+/31417/comment/83d976b8_6e0c796b PS7, Line 1274: if (OSMO_UNLIKELY(rfc5993 && (resp_msg->len != GSM_HR_BYTES_RTP_RFC5993))) { > (The function rtppayload_is_valid sits in the RTP receiving path) […] >> in case both or none of the flags are set both encodings are recognized as valid formats. Are we talking about Schrödinger's cat format here? I don't really want to enter this kind of philosophical topics when looking at code. Make it so that if the flag is set, the BTS lower layers support that format; if not set, it doesn't support it. -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/31417 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I453562da412fde5b928bd2b588129c58ec8e2a7e Gerrit-Change-Number: 31417 Gerrit-PatchSet: 8 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: jolly <andreas(a)eversberg.eu> 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-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Thu, 04 May 2023 15:51:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Comment-In-Reply-To: dexter <pmaier(a)sysmocom.de> Gerrit-MessageType: comment

2 years, 1 month

1
0
0 0

[M] Change in osmo-bts[master]: l1sap: Accept RFC5993 and TS 101.318 HR GSM payload

by dexter

Attention is currently required from: jolly, laforge, pespin, fixeria. dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/31417 ) Change subject: l1sap: Accept RFC5993 and TS 101.318 HR GSM payload ...................................................................... Patch Set 8: (7 comments) Patchset: PS7: > I'm sorry but after a more detailed look, I'm really struggling with this patch. […] I hope it is less confusing now. It is correct that the flags refer to RTP receiving and sending but in this patch we only care about the path that receives RTP packets. The path that sends RTP packets must be handled in a follow up patch. DL: that is correct. In addition to that we also have rtppayload_is_valid() as a gatekeeper to ensure invalid packets are not forwarded to lower layers. UL: that is not in the scope of this patch, it will be added in a follow up patch. File include/osmo-bts/bts.h: https://gerrit.osmocom.org/c/osmo-bts/+/31417/comment/5e5c80ee_0d292e8b PS7, Line 69: /* Whether the BTS model uses HR GSM RTP payload in RFC 5993 format */ > To me this would be "Whether the BTS model supports sending ..." […] (see my other comments, I hope this is clear now.) File src/common/l1sap.c: https://gerrit.osmocom.org/c/osmo-bts/+/31417/comment/b5402a08_22e74794 PS6, Line 1940: if (lchan->type == GSM_LCHAN_TCH_H && rtp_pl_len == GSM_HR_BYTES > please break each condition into one line with uniform formatting. The lines are much shorter now, I think we do not have to break the lines now. https://gerrit.osmocom.org/c/osmo-bts/+/31417/comment/a2faa838_ab48ec48 PS6, Line 1950: } else if (lchan->type == GSM_LCHAN_TCH_H && rtp_pl_len == GSM_HR_BYTES + 1 > The GSM_HR_BYTES_RTP_ constants should be in libosmocore. […] Done File src/common/l1sap.c: https://gerrit.osmocom.org/c/osmo-bts/+/31417/comment/c85625da_11518e5f PS7, Line 1271: bool rfc5993 = bts_internal_flag_get(lchan->ts->trx->bts, BTS_INTERNAL_FLAG_SPEECH_H_V1_RTP_RFC5993); > the description said the internal flag is about "sending", but iiuc you are using it here to decide […] As far as I am aware the flag description says only "uses". Which refers to what the hardware supports (this includes sending and receiving), however in the scope of this patch we only care about which RTP payload format the hardware understands. The comment above says "sending" which is indeed a bit misleading. Let's better use the term "forwarding". https://gerrit.osmocom.org/c/osmo-bts/+/31417/comment/976c91a2_e9ebbf45 PS7, Line 1274: if (OSMO_UNLIKELY(rfc5993 && (resp_msg->len != GSM_HR_BYTES_RTP_RFC5993))) { > so if the BTS supports rfc5993 (sending, receving, whatever, see previous comment), then you check t […] (The function rtppayload_is_valid sits in the RTP receiving path) I probably get now where the confusion is coming from. My idea about the flags was to express a preference. In case both formats are supported none of the two flags would be set. (no check would be carried out, which is probably also not so good) That might be indeed confusing but we can fix this, in case both or none of the flags are set both encodings are recognized as valid formats. I also added a check so that we check with both length if we support both formats. https://gerrit.osmocom.org/c/osmo-bts/+/31417/comment/6a7490ee_0531f2d6 PS7, Line 1280: LOGPLCHAN(lchan, DL1P, LOGL_NOTICE, > There's no need for the "else" here, it is guaranteed by the early return in the previous if. Done -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/31417 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I453562da412fde5b928bd2b588129c58ec8e2a7e Gerrit-Change-Number: 31417 Gerrit-PatchSet: 8 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: jolly <andreas(a)eversberg.eu> 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-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Thu, 04 May 2023 15:43:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Comment-In-Reply-To: dexter <pmaier(a)sysmocom.de> Gerrit-MessageType: comment

2 years, 1 month

1
0
0 0

[M] Change in osmo-bts[master]: l1sap: Accept RFC5993 and TS 101.318 HR GSM payload

by dexter

Attention is currently required from: jolly, laforge, pespin, fixeria. Hello Jenkins Builder, jolly, laforge, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-bts/+/31417 to look at the new patch set (#8). Change subject: l1sap: Accept RFC5993 and TS 101.318 HR GSM payload ...................................................................... l1sap: Accept RFC5993 and TS 101.318 HR GSM payload Unfotunately there are two different RTP formats for HR GSM specified and it is unclear which should be used with GSM networks. Also esch BTS model may have a preference for either one or the other format. Lets set BTS feature flags to determine the preference for each BTS model and then lets use this information to convert incoming RTP packets into the prefered format. Doing so we will make sure that always both formats are accepted. Change-Id: I453562da412fde5b928bd2b588129c58ec8e2a7e Related: OS#5688 --- M include/osmo-bts/bts.h M src/common/bts.c M src/common/l1sap.c M src/osmo-bts-lc15/main.c M src/osmo-bts-oc2g/main.c M src/osmo-bts-sysmo/main.c M src/osmo-bts-trx/main.c M src/osmo-bts-virtual/main.c 8 files changed, 102 insertions(+), 4 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/17/31417/8 -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/31417 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I453562da412fde5b928bd2b588129c58ec8e2a7e Gerrit-Change-Number: 31417 Gerrit-PatchSet: 8 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: jolly <andreas(a)eversberg.eu> 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-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: newpatchset

2 years, 1 month

1
0
0 0

[S] Change in osmo-bts[master]: l1sap: fix wording in comment

by dexter

dexter has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-bts/+/32610 ) Change subject: l1sap: fix wording in comment ...................................................................... l1sap: fix wording in comment The function rtppayload_is_valid() is called from the receiving RTP code path. Lets use the word "forwarding" instead of "sending" to avoid the impression something is sent (like sending RTP packets to the outside world) Change-Id: Ie7fcc53dea462b0d575b0c9ca73ba7507289eefe --- M src/common/l1sap.c 1 file changed, 15 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/10/32610/1 diff --git a/src/common/l1sap.c b/src/common/l1sap.c index 09e2d8c..a7eb830 100644 --- a/src/common/l1sap.c +++ b/src/common/l1sap.c @@ -1255,7 +1255,7 @@ if (resp_msg->len == 0) return false; - /* Avoid sending bw-efficient AMR to lower layers, most bts models + /* Avoid forwarding bw-efficient AMR to lower layers, most bts models * don't support it. */ if (lchan->tch_mode == GSM48_CMODE_SPEECH_AMR && !rtppayload_is_octet_aligned(resp_msg->data, resp_msg->len)) { -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32610 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: Ie7fcc53dea462b0d575b0c9ca73ba7507289eefe Gerrit-Change-Number: 32610 Gerrit-PatchSet: 1 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-MessageType: newchange

2 years, 1 month

1
0
0 0

[S] Change in osmo-iuh[master]: Move testdir src/tests/ -> tests/

by pespin

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-iuh/+/32609 ) Change subject: Move testdir src/tests/ -> tests/ ...................................................................... Move testdir src/tests/ -> tests/ Move tests to usual directory in most projects. Change-Id: Ib9ca59d1dffbbb13fcb9ae2e597b3f114f77524e --- M .gitignore M Makefile.am M configure.ac M src/Makefile.am R tests/Makefile.am R tests/dummy_cn_sua.c R tests/hnb-test-layers.h R tests/hnb-test-ranap.c R tests/hnb-test-rua.c R tests/hnb-test.c R tests/hnb-test.h R tests/test-helpers.c R tests/test-helpers.err R tests/test-helpers.ok R tests/test-hnbap.c R tests/test-hnbap.ok R tests/test-ranap.c R tests/test-ranap.ok R tests/test_common.c R tests/test_common.h R tests/testsuite.at 21 files changed, 29 insertions(+), 18 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-iuh refs/changes/09/32609/1 diff --git a/.gitignore b/.gitignore index 91912b2..209421b 100644 --- a/.gitignore +++ b/.gitignore @@ -21,16 +21,16 @@ Makefile.in Makefile Makefile.am.sample -src/tests/atconfig -src/tests/dummy-cn -src/tests/hnb-test -src/tests/package.m4 -src/tests/test-helpers -src/tests/test-hnbap -src/tests/test-ranap -src/tests/testsuite -src/tests/testsuite.dir -src/tests/testsuite.log +tests/atconfig +tests/dummy-cn +tests/hnb-test +tests/package.m4 +tests/test-helpers +tests/test-hnbap +tests/test-ranap +tests/testsuite +tests/testsuite.dir +tests/testsuite.log src/osmo-hnbgw tags libosmo-hnbap.pc diff --git a/Makefile.am b/Makefile.am index ab3ed63..60fc0b6 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1,6 +1,6 @@ AUTOMAKE_OPTIONS = foreign dist-bzip2 -SUBDIRS = src include doc +SUBDIRS = src include doc tests pkgconfigdir = $(libdir)/pkgconfig pkgconfig_DATA = libosmo-hnbap.pc libosmo-ranap.pc libosmo-rua.pc libosmo-sabp.pc diff --git a/configure.ac b/configure.ac index 5596659..32f0a0d 100644 --- a/configure.ac +++ b/configure.ac @@ -11,7 +11,7 @@ dnl tar-ustar: some asn1 filenames surpass the 99 char limit of tar, so we need dnl to make tar allow longer filenames. -AC_CONFIG_TESTDIR(src/tests) +AC_CONFIG_TESTDIR(tests) CFLAGS="$CFLAGS -std=gnu11" @@ -143,7 +143,7 @@ src/ranap/Makefile src/rua/Makefile src/sabp/Makefile - src/tests/Makefile + tests/Makefile Makefile include/Makefile include/osmocom/Makefile diff --git a/src/Makefile.am b/src/Makefile.am index d217221..378262b 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1,4 +1,4 @@ -SUBDIRS = hnbap rua ranap sabp tests +SUBDIRS = hnbap rua ranap sabp # Build {hnbap,rua,ranap}_{encoder,decoder}.c using asn1tostruct ASN1_ROOT = $(top_srcdir)/asn1 diff --git a/src/tests/Makefile.am b/tests/Makefile.am similarity index 98% rename from src/tests/Makefile.am rename to tests/Makefile.am index 1fca2a4..c08c6d1 100644 --- a/src/tests/Makefile.am +++ b/tests/Makefile.am @@ -1,4 +1,4 @@ -AM_CFLAGS = -g -Wall -I$(top_srcdir)/src/tests \ +AM_CFLAGS = -g -Wall -I$(top_srcdir)/tests \ -I$(top_srcdir)/include -I$(top_builddir)/include \ $(OSMOVTY_CFLAGS) $(OSMOCORE_CFLAGS) $(OSMOGSM_CFLAGS) \ $(OSMONETIF_CFLAGS) $(ASN1C_CFLAGS) $(OSMOSIGTRAN_CFLAGS) diff --git a/src/tests/dummy_cn_sua.c b/tests/dummy_cn_sua.c similarity index 100% rename from src/tests/dummy_cn_sua.c rename to tests/dummy_cn_sua.c diff --git a/src/tests/hnb-test-layers.h b/tests/hnb-test-layers.h similarity index 100% rename from src/tests/hnb-test-layers.h rename to tests/hnb-test-layers.h diff --git a/src/tests/hnb-test-ranap.c b/tests/hnb-test-ranap.c similarity index 100% rename from src/tests/hnb-test-ranap.c rename to tests/hnb-test-ranap.c diff --git a/src/tests/hnb-test-rua.c b/tests/hnb-test-rua.c similarity index 100% rename from src/tests/hnb-test-rua.c rename to tests/hnb-test-rua.c diff --git a/src/tests/hnb-test.c b/tests/hnb-test.c similarity index 100% rename from src/tests/hnb-test.c rename to tests/hnb-test.c diff --git a/src/tests/hnb-test.h b/tests/hnb-test.h similarity index 100% rename from src/tests/hnb-test.h rename to tests/hnb-test.h diff --git a/src/tests/test-helpers.c b/tests/test-helpers.c similarity index 100% rename from src/tests/test-helpers.c rename to tests/test-helpers.c diff --git a/src/tests/test-helpers.err b/tests/test-helpers.err similarity index 100% rename from src/tests/test-helpers.err rename to tests/test-helpers.err diff --git a/src/tests/test-helpers.ok b/tests/test-helpers.ok similarity index 100% rename from src/tests/test-helpers.ok rename to tests/test-helpers.ok diff --git a/src/tests/test-hnbap.c b/tests/test-hnbap.c similarity index 100% rename from src/tests/test-hnbap.c rename to tests/test-hnbap.c diff --git a/src/tests/test-hnbap.ok b/tests/test-hnbap.ok similarity index 100% rename from src/tests/test-hnbap.ok rename to tests/test-hnbap.ok diff --git a/src/tests/test-ranap.c b/tests/test-ranap.c similarity index 100% rename from src/tests/test-ranap.c rename to tests/test-ranap.c diff --git a/src/tests/test-ranap.ok b/tests/test-ranap.ok similarity index 100% rename from src/tests/test-ranap.ok rename to tests/test-ranap.ok diff --git a/src/tests/test_common.c b/tests/test_common.c similarity index 100% rename from src/tests/test_common.c rename to tests/test_common.c diff --git a/src/tests/test_common.h b/tests/test_common.h similarity index 100% rename from src/tests/test_common.h rename to tests/test_common.h diff --git a/src/tests/testsuite.at b/tests/testsuite.at similarity index 63% rename from src/tests/testsuite.at rename to tests/testsuite.at index 9378f18..b0bd68f 100644 --- a/src/tests/testsuite.at +++ b/tests/testsuite.at @@ -6,17 +6,17 @@ AT_KEYWORDS([helpers]) cat $abs_srcdir/test-helpers.ok > expout cat $abs_srcdir/test-helpers.err > experr -AT_CHECK([$abs_top_builddir/src/tests/test-helpers], [0], [expout], [experr]) +AT_CHECK([$abs_top_builddir/tests/test-helpers], [0], [expout], [experr]) AT_CLEANUP AT_SETUP([hnbap]) AT_KEYWORDS([hnbap]) cat $abs_srcdir/test-hnbap.ok > expout -AT_CHECK([$abs_top_builddir/src/tests/test-hnbap], [0], [expout]) +AT_CHECK([$abs_top_builddir/tests/test-hnbap], [0], [expout]) AT_CLEANUP AT_SETUP([ranap]) AT_KEYWORDS([ranap]) cat $abs_srcdir/test-ranap.ok > expout -AT_CHECK([$abs_top_builddir/src/tests/test-ranap], [0], [expout]) +AT_CHECK([$abs_top_builddir/tests/test-ranap], [0], [expout]) AT_CLEANUP -- To view, visit https://gerrit.osmocom.org/c/osmo-iuh/+/32609 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-iuh Gerrit-Branch: master Gerrit-Change-Id: Ib9ca59d1dffbbb13fcb9ae2e597b3f114f77524e Gerrit-Change-Number: 32609 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

2 years, 1 month

1
0
0 0

[S] Change in libosmo-gprs[master]: gmm: Fix false positive compilation error with gcc 13.1.1

by pespin

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-gprs/+/32608 ) Change subject: gmm: Fix false positive compilation error with gcc 13.1.1 ...................................................................... gmm: Fix false positive compilation error with gcc 13.1.1 Newer gcc errors about "cause" being uninitialized, but cause is guaranteed to be set in the "case GPRS_GMM_MS_EV_LOW_LVL_FAIL" path, it just fails to find out. Since the approach used previously is a bit hacky, let's simplify it. Change-Id: I1c96b0aa92d4f9205a1d4d1760c787fe0e0ed169 --- M src/gmm/gmm_ms_fsm.c 1 file changed, 16 insertions(+), 4 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-gprs refs/changes/08/32608/1 diff --git a/src/gmm/gmm_ms_fsm.c b/src/gmm/gmm_ms_fsm.c index 6f82815..f553ad3 100644 --- a/src/gmm/gmm_ms_fsm.c +++ b/src/gmm/gmm_ms_fsm.c @@ -102,7 +102,7 @@ static void st_gmm_ms_registered_initiated(struct osmo_fsm_inst *fi, uint32_t event, void *data) { struct gprs_gmm_ms_fsm_ctx *ctx = (struct gprs_gmm_ms_fsm_ctx *)fi->priv; - uint8_t cause; + uint8_t cause = GMM_CAUSE_MAC_FAIL; int rc; struct gprs_gmm_ms_fsm_attach_ctx att; @@ -120,9 +120,6 @@ memcpy(&att, &ctx->attach, sizeof(att)); gmm_ms_fsm_state_chg(fi, GPRS_GMM_MS_ST_DEREGISTERED); - if (event == GPRS_GMM_MS_EV_LOW_LVL_FAIL) - cause = GMM_CAUSE_MAC_FAIL; - if (att.explicit_att) { /* Submit GMMREG-ATTACH-REJ as per TS 24.007 Annex C.1 */ rc = gprs_gmm_submit_gmmreg_attach_cnf(ctx->gmme, false, cause); -- To view, visit https://gerrit.osmocom.org/c/libosmo-gprs/+/32608 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-gprs Gerrit-Branch: master Gerrit-Change-Id: I1c96b0aa92d4f9205a1d4d1760c787fe0e0ed169 Gerrit-Change-Number: 32608 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

2 years, 1 month

1
0
0 0

[S] Change in osmo-bts[master]: octphy: Fix clearly wrong noop assignment

by pespin

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/32602 ) Change subject: octphy: Fix clearly wrong noop assignment ...................................................................... octphy: Fix clearly wrong noop assignment This code has been there since first octphy support was added. New gcc 13.1.1 is catching this and reporting compilation errors: """ /osmo-bts/src/osmo-bts-octphy/l1_oml.c: In function ‘ts_connect_as’: /osmo-bts/src/osmo-bts-octphy/l1_oml.c:1518:60: error: ‘oc’ is used uninitialized [-Werror=uninitialized] 1518 | tOCTVC1_GSM_MSG_TRX_ACTIVATE_PHYSICAL_CHANNEL_CMD *oc = | ^~ /osmo-bts/src/osmo-bts-octphy/l1_oml.c:1518:60: note: ‘oc’ was declared here 1518 | tOCTVC1_GSM_MSG_TRX_ACTIVATE_PHYSICAL_CHANNEL_CMD *oc = | ^~ /osmo-bts/src/osmo-bts-octphy/l1_oml.c: In function ‘bts_model_ts_disconnect’: /osmo-bts/src/osmo-bts-octphy/l1_oml.c:1803:62: error: ‘oc’ is used uninitialized [-Werror=uninitialized] 1803 | tOCTVC1_GSM_MSG_TRX_DEACTIVATE_PHYSICAL_CHANNEL_CMD *oc = | ^~ /osmo-bts/src/osmo-bts-octphy/l1_oml.c:1803:62: note: ‘oc’ was declared here 1803 | tOCTVC1_GSM_MSG_TRX_DEACTIVATE_PHYSICAL_CHANNEL_CMD *oc = | ^~ """ Change-Id: I0da7d3a94e9eba15b8d3d3d995bba532170d9df7 --- M src/osmo-bts-octphy/l1_oml.c 1 file changed, 29 insertions(+), 4 deletions(-) Approvals: fixeria: Looks good to me, approved Jenkins Builder: Verified diff --git a/src/osmo-bts-octphy/l1_oml.c b/src/osmo-bts-octphy/l1_oml.c index 28fb60c..bb519a0 100644 --- a/src/osmo-bts-octphy/l1_oml.c +++ b/src/osmo-bts-octphy/l1_oml.c @@ -1515,8 +1515,7 @@ struct phy_instance *pinst = trx_phy_instance(ts->trx); struct octphy_hdl *fl1h = pinst->phy_link->u.octphy.hdl; struct msgb *msg = l1p_msgb_alloc(); - tOCTVC1_GSM_MSG_TRX_ACTIVATE_PHYSICAL_CHANNEL_CMD *oc = - (tOCTVC1_GSM_MSG_TRX_ACTIVATE_PHYSICAL_CHANNEL_CMD *) oc; + tOCTVC1_GSM_MSG_TRX_ACTIVATE_PHYSICAL_CHANNEL_CMD *oc; oc = (tOCTVC1_GSM_MSG_TRX_ACTIVATE_PHYSICAL_CHANNEL_CMD*) msgb_put(msg, sizeof(*oc)); @@ -1800,8 +1799,7 @@ struct phy_instance *pinst = trx_phy_instance(ts->trx); struct octphy_hdl *fl1h = pinst->phy_link->u.octphy.hdl; struct msgb *msg = l1p_msgb_alloc(); - tOCTVC1_GSM_MSG_TRX_DEACTIVATE_PHYSICAL_CHANNEL_CMD *oc = - (tOCTVC1_GSM_MSG_TRX_DEACTIVATE_PHYSICAL_CHANNEL_CMD *) oc; + tOCTVC1_GSM_MSG_TRX_DEACTIVATE_PHYSICAL_CHANNEL_CMD *oc; oc = (tOCTVC1_GSM_MSG_TRX_DEACTIVATE_PHYSICAL_CHANNEL_CMD *) msgb_put(msg, sizeof(*oc)); -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32602 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I0da7d3a94e9eba15b8d3d3d995bba532170d9df7 Gerrit-Change-Number: 32602 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

2 years, 1 month

1
0
0 0

[M] Change in osmo-ttcn3-hacks[master]: PCUIF_Types: add templates for PCU_IF_MSG_DATA_CNF_DT message types

by pespin

Attention is currently required from: dexter. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/32607 ) Change subject: PCUIF_Types: add templates for PCU_IF_MSG_DATA_CNF_DT message types ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/32607 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: Iccde71e1dd6fbd421652c5892bc2c1f32a8535ff Gerrit-Change-Number: 32607 Gerrit-PatchSet: 1 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Thu, 04 May 2023 14:08:31 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 1 month

1
0
0 0

2025

2024

2023

2022

gerrit-log May 2023