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

[M] Change in libosmocore[master]: gsm: add osmo_mobile_identity_decode_from_l3_buf()

by daniel

Attention is currently required from: neels. daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/32577 ) Change subject: gsm: add osmo_mobile_identity_decode_from_l3_buf() ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/32577 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I9bd99ccd01f0eedc091fe51687ff92ae1fdff60b Gerrit-Change-Number: 32577 Gerrit-PatchSet: 1 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Comment-Date: Thu, 04 May 2023 17:37:59 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

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 daniel

Attention is currently required from: osmith, pespin, fixeria. daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-gprs/+/32608 ) Change subject: gmm: Fix false positive compilation error with gcc 13.1.1 ...................................................................... Patch Set 1: Code-Review+1 -- 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-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Thu, 04 May 2023 17:36:32 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 1 month

1
0
0 0

[S] Change in osmo-ttcn3-hacks[master]: PCUIF_Types: rename PCU_IF_SAPI_AGCH_DT to PCU_IF_SAPI_PCH_DT

by fixeria

Attention is currently required from: dexter. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/32604 ) Change subject: PCUIF_Types: rename PCU_IF_SAPI_AGCH_DT to PCU_IF_SAPI_PCH_DT ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/32604 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: Ifc09067bcb0f9f422ca429313fa09fea081dc316 Gerrit-Change-Number: 32604 Gerrit-PatchSet: 1 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Thu, 04 May 2023 17:32:28 +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 neels

Attention is currently required from: jolly, laforge, pespin, fixeria, dexter. neels 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: (9 comments) Commit Message: https://gerrit.osmocom.org/c/osmo-bts/+/31417/comment/39f49542_c4452fbe PS8, Line 16: are accepted. I am not clear here, there is code in this patch that converts the HR format to match the BTS, and there is also code that rejects an RTP packet when the HR format does not match the BTS. AFAICT we should need only one of these. Could you please clarify the intention of this patch? File src/common/bts.c: https://gerrit.osmocom.org/c/osmo-bts/+/31417/comment/72c3c35d_453b25eb PS8, Line 207: " add "...is supported" or "support for ..." https://gerrit.osmocom.org/c/osmo-bts/+/31417/comment/02db462e_9a563be5 PS8, Line 208: t same File src/common/l1sap.c: https://gerrit.osmocom.org/c/osmo-bts/+/31417/comment/8682887e_df3ca21b PS7, Line 1274: if (OSMO_UNLIKELY(rfc5993 && (resp_msg->len != GSM_HR_BYTES_RTP_RFC5993))) { > >> in case both or none of the flags are set both encodings are recognized as valid formats. […] I agree that a flag being zero should disallow that format, no matter what the other flag's value is. I'd also ask Pau please to reflect on particular wording choices in your code review comments. We are aiming for a supportive atmosphere that invites developers. I'm pretty sure you are commenting with a friendly wink, but these easily go unnoticed in a written medium. Thanks! =) File src/common/l1sap.c: https://gerrit.osmocom.org/c/osmo-bts/+/31417/comment/815cffbb_2ae703df 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 M […] AFAICT the function name is not part of this patch. you are welcome to submit your own patch to improve the naming? https://gerrit.osmocom.org/c/osmo-bts/+/31417/comment/881ccae6_9c20663f 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: […] I agree with the patch here, to keep the logic of this function uniform: "return false if anything is wrong, and otherwise carry on to the final 'return true'". Possibly the incoming packet is not HR at all, and we only exit with false if it is HR and also the format is not supported. Also agree with the patch in adding concise logging. But I think as in earlier comments the conditions here should be if (len == GSM_HR_BYTES_FOO && !foo_supported) { LOG("dropping unsupported RTP payload format foo") return false; } (two of these, one for each format) EDIT: but I am confused about whether this patch is instead converting to another format, then this part here can be dropped? https://gerrit.osmocom.org/c/osmo-bts/+/31417/comment/385a8d55_9ddeec5a PS8, Line 1275: if (OSMO_UNLIKELY(rfc5993 && !ts101318 && (resp_msg->len != GSM_HR_BYTES_RTP_RFC5993))) { I'm still trying to understand OSMO_UNLIKELY() -- seems to me that this is not a good use case for OSMO_UNLIKELY()? if anyone can explain that would be welcome. https://gerrit.osmocom.org/c/osmo-bts/+/31417/comment/eda2b1d9_6bab0c0e 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". […] need clarification: is this patch converting or rejecting? https://gerrit.osmocom.org/c/osmo-bts/+/31417/comment/95b98f55_11997f15 PS8, Line 1969: break; the code converting from one format to the other should be in separate functions -- 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-CC: neels <nhofmeyr(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-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Thu, 04 May 2023 17:00:22 +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

[S] Change in libasn1c[master]: Disable _ASN_STACK_OVERFLOW_CHECK if building with Asan enabled

by pespin

Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libasn1c/+/32612 to look at the new patch set (#2). Change subject: Disable _ASN_STACK_OVERFLOW_CHECK if building with Asan enabled ...................................................................... Disable _ASN_STACK_OVERFLOW_CHECK if building with Asan enabled Change-Id: I2dda4720f3ea5a023d340863db177e6334beeaa3 --- M include/asn1c/asn_internal.h 1 file changed, 35 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libasn1c refs/changes/12/32612/2 -- To view, visit https://gerrit.osmocom.org/c/libasn1c/+/32612 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libasn1c Gerrit-Branch: master Gerrit-Change-Id: I2dda4720f3ea5a023d340863db177e6334beeaa3 Gerrit-Change-Number: 32612 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-MessageType: newpatchset

2 years, 1 month

1
0
0 0

[S] Change in libasn1c[master]: .gitignore: Add configure~

by pespin

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libasn1c/+/32611 ) Change subject: .gitignore: Add configure~ ...................................................................... .gitignore: Add configure~ Change-Id: Ie46723fe3ad5b27813c102e068f433215692271f --- M .gitignore 1 file changed, 10 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libasn1c refs/changes/11/32611/1 diff --git a/.gitignore b/.gitignore index 2852b4d..c7ac391 100644 --- a/.gitignore +++ b/.gitignore @@ -8,6 +8,7 @@ aclocal.m4 config.* configure +configure~ compile depcomp libtool -- To view, visit https://gerrit.osmocom.org/c/libasn1c/+/32611 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libasn1c Gerrit-Branch: master Gerrit-Change-Id: Ie46723fe3ad5b27813c102e068f433215692271f Gerrit-Change-Number: 32611 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

2 years, 1 month

1
0
0 0

[S] Change in libasn1c[master]: Disable _ASN_STACK_OVERFLOW_CHECK if building with Asan enabled

by pespin

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libasn1c/+/32612 ) Change subject: Disable _ASN_STACK_OVERFLOW_CHECK if building with Asan enabled ...................................................................... Disable _ASN_STACK_OVERFLOW_CHECK if building with Asan enabled Change-Id: I2dda4720f3ea5a023d340863db177e6334beeaa3 --- M include/asn1c/asn_internal.h 1 file changed, 32 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libasn1c refs/changes/12/32612/1 diff --git a/include/asn1c/asn_internal.h b/include/asn1c/asn_internal.h index 580c2e7..f5f6282 100644 --- a/include/asn1c/asn_internal.h +++ b/include/asn1c/asn_internal.h @@ -108,9 +108,31 @@ /* * Check stack against overflow, if limit is set. */ + +/* Since GCC 13, AddressSanitizer started defaulting to +* ASAN_OPTIONS="detect_stack_use_after_return=1", which makes this check +* fail due to apparently jumping stack pointers. +* Hence, disable this check if building with ASan, as documented in: +* GCC: https://gcc.gnu.org/onlinedocs/cpp/Common-Predefined-Macros.html +* Clang: https://clang.llvm.org/docs/AddressSanitizer.html#conditional-compilation-w… +*/ +#if defined(__SANITIZE_ADDRESS__) + #define _ASN_SANITIZE_ENABLED 1 +#elif defined(__has_feature) +#if __has_feature(address_sanitizer) + #define _ASN_SANITIZE_ENABLED 1 +#endif +#endif + #define _ASN_DEFAULT_STACK_MAX (30000) + +#if defined(_ASN_SANITIZE_ENABLED) +static inline int +_ASN_STACK_OVERFLOW_CHECK(asn_codec_ctx_t *ctx) { return 0; } +#else static inline int _ASN_STACK_OVERFLOW_CHECK(asn_codec_ctx_t *ctx) { + if(ctx && ctx->max_stack_size) { /* ctx MUST be allocated on the stack */ @@ -126,6 +148,7 @@ } return 0; } +#endif #ifdef __cplusplus } -- To view, visit https://gerrit.osmocom.org/c/libasn1c/+/32612 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libasn1c Gerrit-Branch: master Gerrit-Change-Id: I2dda4720f3ea5a023d340863db177e6334beeaa3 Gerrit-Change-Number: 32612 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]: l1sap: fix wording in comment

by neels

Attention is currently required from: dexter. neels 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+2 (1 comment) Patchset: PS1: for me "sending to lower layers" is a dead giveaway that this is vertical, but ok if it helps you -- 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: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Thu, 04 May 2023 16:17:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 1 month

1
0
0 0

[S] Change in libosmocore[master]: vty: fix vty->index for implicit go_parent_node

by neels

Attention is currently required from: pespin, fixeria. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/32562 ) Change subject: vty: fix vty->index for implicit go_parent_node ...................................................................... Patch Set 2: (1 comment) File include/osmocom/vty/vty.h: https://gerrit.osmocom.org/c/libosmocore/+/32562/comment/a8d13fc9_8f8760b1 PS1, Line 64: void *index; > https://gerrit.osmocom. […] so fixeria if you agree with the patch followin that moves the struct to private .c i'll mark this resolved -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/32562 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Id408c678d18ba19b1c1394c3fb657536153d2094 Gerrit-Change-Number: 32562 Gerrit-PatchSet: 2 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Thu, 04 May 2023 16:14:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: comment

2 years, 1 month

1
0
0 0

[M] Change in libosmocore[master]: vty: show bug in implicit go_parent_node

by neels

Attention is currently required from: laforge, pespin, fixeria. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/32561 ) Change subject: vty: show bug in implicit go_parent_node ...................................................................... Patch Set 2: (2 comments) Commit Message: https://gerrit.osmocom.org/c/libosmocore/+/32561/comment/93f51fd2_cde9277b PS1, Line 26: there is no legacy vty_go_parent() function that sets the correct : index via a switch(). > yes, for example in cs7 config, the go_parent_cb() does the action of applying config changes. […] Done File tests/vty/vty_transcript_test.c: https://gerrit.osmocom.org/c/libosmocore/+/32561/comment/8cc41617_94b64364 PS1, Line 344: talloc_asprintf(root_ctx, argv[0]); > yes, seems i wasn't paying attention, thx Done -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/32561 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I2472daed7436a1947655b06d34eb217e595bc7f3 Gerrit-Change-Number: 32561 Gerrit-PatchSet: 2 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> 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 16:12:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de> Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: comment

2 years, 1 month

1
0
0 0

2025

2024

2023

2022

gerrit-log May 2023