January 2024 - gerrit-log - lists.osmocom.org

[M] Change in osmo-mgw[master]: Revert "drop (now) unused code"
by neels 11 Jan '24

11 Jan '24

Attention is currently required from: laforge, pespin. Hello Jenkins Builder, laforge, pespin, I'd like you to do a code review. Please visit https://gerrit.osmocom.org/c/osmo-mgw/+/35526?usp=email to review the following change. Change subject: Revert "drop (now) unused code" ...................................................................... Revert "drop (now) unused code" This reverts commit 2b30cbdfa83c20045ef92f53e88441dda185591b. Reason for revert: Older versions of osmo-msc were actually calling map_codec_to_pt(). Change-Id: Ifff31012b327d40ed0b1559d5cf4f320784a4061 Related: https://jenkins.osmocom.org/jenkins/job/Osmocom-build-tags-against-master/1… --- M TODO-RELEASE M include/osmocom/mgcp_client/mgcp_client.h M src/libosmo-mgcp-client/mgcp_client.c M tests/mgcp_client/mgcp_client_test.c M tests/mgcp_client/mgcp_client_test.err M tests/mgcp_client/mgcp_client_test.ok 6 files changed, 191 insertions(+), 5 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/26/35526/1 diff --git a/TODO-RELEASE b/TODO-RELEASE index f1a702f..964ffe1 100644 --- a/TODO-RELEASE +++ b/TODO-RELEASE @@ -38,8 +38,3 @@ libosmo-mgcp-client deprecate public API New code should no longer use codecs[], instead use ptmap[].codec. There is backwards compat code that moves codecs[] entries, if any, over to ptmap[], so callers may migrate at own leisure. -libosmo-mgcp-client remove public API Since codecs[] has been deprecated in favor of ptmap[], there is no use - for the following functions; all known callers have been within - osmo-mgw.git: - map_codec_to_pt() - map_pt_to_codec() diff --git a/include/osmocom/mgcp_client/mgcp_client.h b/include/osmocom/mgcp_client/mgcp_client.h index e1748a6..1d33690 100644 --- a/include/osmocom/mgcp_client/mgcp_client.h +++ b/include/osmocom/mgcp_client/mgcp_client.h @@ -119,5 +119,9 @@ } enum mgcp_codecs map_str_to_codec(const char *str); +unsigned int map_codec_to_pt(const struct ptmap *ptmap, unsigned int ptmap_len, + enum mgcp_codecs codec); +enum mgcp_codecs map_pt_to_codec(struct ptmap *ptmap, unsigned int ptmap_len, + unsigned int pt); const char *mgcp_client_name(const struct mgcp_client *mgcp); diff --git a/src/libosmo-mgcp-client/mgcp_client.c b/src/libosmo-mgcp-client/mgcp_client.c index bfe69e6..489ce69 100644 --- a/src/libosmo-mgcp-client/mgcp_client.c +++ b/src/libosmo-mgcp-client/mgcp_client.c @@ -105,6 +105,94 @@ return -1; } +/* Check the ptmap for illegal mappings */ +static int check_ptmap(const struct ptmap *ptmap) +{ + /* Check if there are mappings that leave the IANA assigned dynamic + * payload type range. Under normal conditions such mappings should + * not occur */ + + /* Its ok to have a 1:1 mapping in the statically defined + * range, this won't hurt */ + if (ptmap->codec == ptmap->pt) + return 0; + + if (ptmap->codec < 96 || ptmap->codec > 127) + goto error; + if (ptmap->pt < 96 || ptmap->pt > 127) + goto error; + + return 0; +error: + LOGP(DLMGCP, LOGL_ERROR, + "ptmap contains illegal mapping: codec=%u maps to pt=%u\n", + ptmap->codec, ptmap->pt); + return -1; +} + +/*! Map a codec to a payload type. + * \ptmap[in] payload pointer to payload type map with specified payload types. + * \ptmap[in] ptmap_len length of the payload type map. + * \ptmap[in] codec the codec for which the payload type should be looked up. + * \returns assigned payload type */ +unsigned int map_codec_to_pt(const struct ptmap *ptmap, unsigned int ptmap_len, + enum mgcp_codecs codec) +{ + unsigned int i; + + /*! Note: If the payload type map is empty or the codec is not found + * in the map, then a 1:1 mapping is performed. If the codec falls + * into the statically defined range or if the mapping table isself + * tries to map to the statically defined range, then the mapping + * is also ignored and a 1:1 mapping is performed instead. */ + + /* we may return the codec directly since enum mgcp_codecs directly + * corresponds to the statically assigned payload types */ + if (codec < 96 || codec > 127) + return codec; + + for (i = 0; i < ptmap_len; i++) { + /* Skip illegal map entries */ + if (check_ptmap(ptmap) == 0 && ptmap->codec == codec) + return ptmap->pt; + ptmap++; + } + + /* If nothing is found, do not perform any mapping */ + return codec; +} + +/*! Map a payload type to a codec. + * \ptmap[in] payload pointer to payload type map with specified payload types. + * \ptmap[in] ptmap_len length of the payload type map. + * \ptmap[in] payload type for which the codec should be looked up. + * \returns codec that corresponds to the specified payload type */ +enum mgcp_codecs map_pt_to_codec(struct ptmap *ptmap, unsigned int ptmap_len, + unsigned int pt) +{ + unsigned int i; + + /*! Note: If the payload type map is empty or the payload type is not + * found in the map, then a 1:1 mapping is performed. If the payload + * type falls into the statically defined range or if the mapping + * table isself tries to map to the statically defined range, then + * the mapping is also ignored and a 1:1 mapping is performed + * instead. */ + + /* See also note in map_codec_to_pt() */ + if (pt < 96 || pt > 127) + return pt; + + for (i = 0; i < ptmap_len; i++) { + if (check_ptmap(ptmap) == 0 && ptmap->pt == pt) + return ptmap->codec; + ptmap++; + } + + /* If nothing is found, do not perform any mapping */ + return pt; +} + static void _mgcp_client_conf_init(struct mgcp_client_conf *conf) { /* NULL and -1 default to MGCP_CLIENT_*_DEFAULT values */ diff --git a/tests/mgcp_client/mgcp_client_test.c b/tests/mgcp_client/mgcp_client_test.c index 95a742f..9b356c3 100644 --- a/tests/mgcp_client/mgcp_client_test.c +++ b/tests/mgcp_client/mgcp_client_test.c @@ -571,6 +571,57 @@ OSMO_ASSERT(map_str_to_codec("AMR-WB####################################################################################################################") == -1); } +static void test_map_codec_to_pt_and_map_pt_to_codec(void) +{ + struct ptmap ptmap[10]; + unsigned int ptmap_len; + unsigned int i; + + ptmap[0].codec = CODEC_GSMEFR_8000_1; + ptmap[0].pt = 96; + ptmap[1].codec = CODEC_GSMHR_8000_1; + ptmap[1].pt = 97; + ptmap[2].codec = CODEC_AMR_8000_1; + ptmap[2].pt = 98; + ptmap[3].codec = CODEC_AMRWB_16000_1; + ptmap[3].pt = 99; + ptmap_len = 4; + + /* Mappings that are covered by the table */ + for (i = 0; i < ptmap_len; i++) + printf(" %u => %u\n", ptmap[i].codec, map_codec_to_pt(ptmap, ptmap_len, ptmap[i].codec)); + for (i = 0; i < ptmap_len; i++) + printf(" %u <= %u\n", ptmap[i].pt, map_pt_to_codec(ptmap, ptmap_len, ptmap[i].pt)); + printf("\n"); + + /* Map some codecs/payload types from the static range, result must + * always be a 1:1 mapping */ + printf(" %u => %u\n", CODEC_PCMU_8000_1, map_codec_to_pt(ptmap, ptmap_len, CODEC_PCMU_8000_1)); + printf(" %u => %u\n", CODEC_GSM_8000_1, map_codec_to_pt(ptmap, ptmap_len, CODEC_GSM_8000_1)); + printf(" %u => %u\n", CODEC_PCMA_8000_1, map_codec_to_pt(ptmap, ptmap_len, CODEC_PCMA_8000_1)); + printf(" %u => %u\n", CODEC_G729_8000_1, map_codec_to_pt(ptmap, ptmap_len, CODEC_G729_8000_1)); + printf(" %u <= %u\n", CODEC_PCMU_8000_1, map_pt_to_codec(ptmap, ptmap_len, CODEC_PCMU_8000_1)); + printf(" %u <= %u\n", CODEC_GSM_8000_1, map_pt_to_codec(ptmap, ptmap_len, CODEC_GSM_8000_1)); + printf(" %u <= %u\n", CODEC_PCMA_8000_1, map_pt_to_codec(ptmap, ptmap_len, CODEC_PCMA_8000_1)); + printf(" %u <= %u\n", CODEC_G729_8000_1, map_pt_to_codec(ptmap, ptmap_len, CODEC_G729_8000_1)); + printf("\n"); + + /* Try to do mappings from statically defined range to danymic range and vice versa. This + * is illegal and should result into a 1:1 mapping */ + ptmap[3].codec = CODEC_AMRWB_16000_1; + ptmap[3].pt = 2; + ptmap[4].codec = CODEC_PCMU_8000_1; + ptmap[4].pt = 100; + ptmap_len = 5; + + /* Apply all mappings again, the illegal ones we defined should result into 1:1 mappings */ + for (i = 0; i < ptmap_len; i++) + printf(" %u => %u\n", ptmap[i].codec, map_codec_to_pt(ptmap, ptmap_len, ptmap[i].codec)); + for (i = 0; i < ptmap_len; i++) + printf(" %u <= %u\n", ptmap[i].pt, map_pt_to_codec(ptmap, ptmap_len, ptmap[i].pt)); + printf("\n"); +} + void test_mgcp_client_e1_epname(void) { char *epname; @@ -857,6 +908,7 @@ test_mgcp_msg(); test_mgcp_client_cancel(); test_sdp_section_start(); + test_map_codec_to_pt_and_map_pt_to_codec(); test_map_str_to_codec(); test_mgcp_client_e1_epname(); diff --git a/tests/mgcp_client/mgcp_client_test.err b/tests/mgcp_client/mgcp_client_test.err index 9e98792..94fe351 100644 --- a/tests/mgcp_client/mgcp_client_test.err +++ b/tests/mgcp_client/mgcp_client_test.err @@ -128,6 +128,10 @@ body: "some mgcp header data\r\nand header params\r\n\r\nc=IN IP4 \r\n" DLMGCP Failed to parse MGCP response header (audio ip) got rc=-22 +DLMGCP ptmap contains illegal mapping: codec=113 maps to pt=2 +DLMGCP ptmap contains illegal mapping: codec=0 maps to pt=100 +DLMGCP ptmap contains illegal mapping: codec=113 maps to pt=2 +DLMGCP ptmap contains illegal mapping: codec=0 maps to pt=100 DLMGCP MGW(mgw) MGCP client: using endpoint domain '@mgw' DLMGCP MGW(mgw) Cannot compose MGCP e1-endpoint name (ds/e1-15/s-1/su128-0@mgw), rate(128)/offset(0) combination is invalid! DLMGCP MGW(mgw) Cannot compose MGCP e1-endpoint name (ds/e1-15/s-1/su8-16@mgw), rate(8)/offset(16) combination is invalid! diff --git a/tests/mgcp_client/mgcp_client_test.ok b/tests/mgcp_client/mgcp_client_test.ok index b16d1bc..039fbd9 100644 --- a/tests/mgcp_client/mgcp_client_test.ok +++ b/tests/mgcp_client/mgcp_client_test.ok @@ -181,6 +181,35 @@ test_sdp_section_start() test [17]: test_sdp_section_start() test [18]: + 110 => 96 + 111 => 97 + 112 => 98 + 113 => 99 + 96 <= 110 + 97 <= 111 + 98 <= 112 + 99 <= 113 + + 0 => 0 + 3 => 3 + 8 => 8 + 18 => 18 + 0 <= 0 + 3 <= 3 + 8 <= 8 + 18 <= 18 + + 110 => 96 + 111 => 97 + 112 => 98 + 113 => 113 + 0 => 0 + 96 <= 110 + 97 <= 111 + 98 <= 112 + 2 <= 2 + 100 <= 100 + ds/e1-1/s-15/su64-0@mgw ds/e1-2/s-14/su32-0@mgw ds/e1-3/s-13/su32-4@mgw -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35526?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Ifff31012b327d40ed0b1559d5cf4f320784a4061 Gerrit-Change-Number: 35526 Gerrit-PatchSet: 1 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

1 0

[M] Change in libosmocore[master]: Revert "Revert "logging: add log level cache""
by Hoernchen 11 Jan '24

11 Jan '24

Attention is currently required from: fixeria, laforge. Hoernchen has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35525?usp=email ) Change subject: Revert "Revert "logging: add log level cache"" ...................................................................... Patch Set 1: (1 comment) Commit Message: https://gerrit.osmocom.org/c/libosmocore/+/35525/comment/6f5c6ff1_033c8328 PS1, Line 11: some fallout expected > Sure, we can fix the unit tests in master of the related packages. […] But that promise is impossible to keep because that means 11 year old test code that checks the number of talloc chunks makes it impossible to change any related code because our memory layout is fixed forever and it would stop building any time the chunk number changes... Test code can't be part of that promise and I am surprised to hear that this is apparently the case. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35525?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I539872fc9e3c50b407e6bc388f1e091fa2c826c3 Gerrit-Change-Number: 35525 Gerrit-PatchSet: 1 Gerrit-Owner: Hoernchen <ewild(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: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Thu, 11 Jan 2024 18:27:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Hoernchen <ewild(a)sysmocom.de> Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[S] Change in osmo-bts[master]: test: adjust for libosmocore loglevel cache
by fixeria 11 Jan '24

11 Jan '24

Attention is currently required from: Hoernchen. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/35550?usp=email ) Change subject: test: adjust for libosmocore loglevel cache ...................................................................... Patch Set 1: (1 comment) File tests/handover/handover_test.c: https://gerrit.osmocom.org/c/osmo-bts/+/35550/comment/da8fe317_dfcda4ff PS1, Line 73: osmo_stderr_target->categories[DHO].loglevel = LOGL_DEBUG; > Because this unrelated I disagree. This is related. By using the right API we kill two birds with one stone: fix accessing logging internals and ensure that the cache is properly updated. If these tests used `log_set_category_filter()`, we would not end up having those regressions... > test code is always weird/old and rarely does things normal code "should" do things. I am not getting the point here. Yes, tests may be receiving less care from developers and do something improperly, but does it mean that we should not fix tests? -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/35550?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: Ie9c546210bc554843b1029afa20965bde1d7f228 Gerrit-Change-Number: 35550 Gerrit-PatchSet: 1 Gerrit-Owner: Hoernchen <ewild(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: Hoernchen <ewild(a)sysmocom.de> Gerrit-Comment-Date: Thu, 11 Jan 2024 18:17:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Hoernchen <ewild(a)sysmocom.de> Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[S] Change in osmo-bsc[master]: test: adjust for libosmocore loglevel cache
by laforge 11 Jan '24

11 Jan '24

Attention is currently required from: Hoernchen. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/35549?usp=email ) Change subject: test: adjust for libosmocore loglevel cache ...................................................................... Patch Set 1: Code-Review-1 (1 comment) Patchset: PS1: *depending* on a certain libosmocore fix is not a solution, as explained there. won't help for old applications. -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/35549?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: If52bcb522d41918db9e0755ead58abdcb8f0209b Gerrit-Change-Number: 35549 Gerrit-PatchSet: 1 Gerrit-Owner: Hoernchen <ewild(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: Hoernchen <ewild(a)sysmocom.de> Gerrit-Comment-Date: Thu, 11 Jan 2024 18:17:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in osmo-bts[master]: test: adjust for libosmocore loglevel cache
by laforge 11 Jan '24

11 Jan '24

Attention is currently required from: Hoernchen, fixeria. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/35550?usp=email ) Change subject: test: adjust for libosmocore loglevel cache ...................................................................... Patch Set 1: Code-Review-1 (1 comment) Patchset: PS1: *depending* on a certain libosmocore fix is not a solution, as explained there. won't help for old applications. -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/35550?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: Ie9c546210bc554843b1029afa20965bde1d7f228 Gerrit-Change-Number: 35550 Gerrit-PatchSet: 1 Gerrit-Owner: Hoernchen <ewild(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: Hoernchen <ewild(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Thu, 11 Jan 2024 18:16:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in libosmocore[master]: Revert "Revert "logging: add log level cache""
by laforge 11 Jan '24

11 Jan '24

Attention is currently required from: Hoernchen, fixeria. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35525?usp=email ) Change subject: Revert "Revert "logging: add log level cache"" ...................................................................... Patch Set 1: (1 comment) Commit Message: https://gerrit.osmocom.org/c/libosmocore/+/35525/comment/764b4dc6_4d9edc4b PS1, Line 11: some fallout expected > This only affects the two other packages, at least as far as "make check" is concerned... […] Sure, we can fix the unit tests in master of the related packages. The problem is that we always have to think of building old application versions against later versions of the library. That's the kind of backwards-compatibility that we've been promising. I don't see why it would mean we should abandon the cache in general. The proposed solution with explicitly enabling it is certainly an option. Everyone who cares about performance will enable it, and old code continues to behave the same way as it does so far. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35525?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I539872fc9e3c50b407e6bc388f1e091fa2c826c3 Gerrit-Change-Number: 35525 Gerrit-PatchSet: 1 Gerrit-Owner: Hoernchen <ewild(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: Hoernchen <ewild(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Thu, 11 Jan 2024 18:12:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Hoernchen <ewild(a)sysmocom.de> Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[S] Change in osmo-bts[master]: test: adjust for libosmocore loglevel cache
by Hoernchen 11 Jan '24

11 Jan '24

Attention is currently required from: fixeria. Hoernchen has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/35550?usp=email ) Change subject: test: adjust for libosmocore loglevel cache ...................................................................... Patch Set 1: (1 comment) File tests/handover/handover_test.c: https://gerrit.osmocom.org/c/osmo-bts/+/35550/comment/31430458_4d1a7579 PS1, Line 73: osmo_stderr_target->categories[DHO].loglevel = LOGL_DEBUG; > So, if accessing the logging internals is a bad idea (as you mentioned in the related patch), then w […] Because this unrelated and it is test code, test code is always weird/old and rarely does things normal code "should" do things. In theory callers of the cache update func "should" hold a vty lock, but this does not matter in this case, because there can't really be any other threads... -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/35550?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: Ie9c546210bc554843b1029afa20965bde1d7f228 Gerrit-Change-Number: 35550 Gerrit-PatchSet: 1 Gerrit-Owner: Hoernchen <ewild(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Thu, 11 Jan 2024 18:00:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[M] Change in libosmocore[master]: Revert "Revert "logging: add log level cache""
by Hoernchen 11 Jan '24

11 Jan '24

Attention is currently required from: fixeria, laforge. Hoernchen has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35525?usp=email ) Change subject: Revert "Revert "logging: add log level cache"" ...................................................................... Patch Set 1: (1 comment) Commit Message: https://gerrit.osmocom.org/c/libosmocore/+/35525/comment/b778a57c_7ec007f1 PS1, Line 11: some fallout expected > If we revert the revert, we end up with the same problem: old code (mostly tests, but who knows) usi […] This only affects the two other packages, at least as far as "make check" is concerned... so we either fix anything else right now or we abandon the cache. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35525?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I539872fc9e3c50b407e6bc388f1e091fa2c826c3 Gerrit-Change-Number: 35525 Gerrit-PatchSet: 1 Gerrit-Owner: Hoernchen <ewild(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: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Thu, 11 Jan 2024 17:57:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[M] Change in libosmocore[master]: Revert "Revert "logging: add log level cache""
by fixeria 11 Jan '24

11 Jan '24

Attention is currently required from: Hoernchen, laforge. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35525?usp=email ) Change subject: Revert "Revert "logging: add log level cache"" ...................................................................... Patch Set 1: (1 comment) Commit Message: https://gerrit.osmocom.org/c/libosmocore/+/35525/comment/a45af4fa_ffec6c8e PS1, Line 11: some fallout expected If we revert the revert, we end up with the same problem: old code (mostly tests, but who knows) using improper API to manipulate the logging configuration may suddenly stop logging because the cache if not properly maintained. I don't have a strong opinion here, but to me it looks like a substantial side effect, which can be avoided. A potential solution would be not enabling the cache by default and requiring the API user to explicitly enable it, by calling `osmo_log_cache_enable()` for instance. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35525?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I539872fc9e3c50b407e6bc388f1e091fa2c826c3 Gerrit-Change-Number: 35525 Gerrit-PatchSet: 1 Gerrit-Owner: Hoernchen <ewild(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: Hoernchen <ewild(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Thu, 11 Jan 2024 17:51:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in osmo-bts[master]: test: adjust for libosmocore loglevel cache
by fixeria 11 Jan '24

11 Jan '24

Attention is currently required from: Hoernchen. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/35550?usp=email ) Change subject: test: adjust for libosmocore loglevel cache ...................................................................... Patch Set 1: (1 comment) File tests/handover/handover_test.c: https://gerrit.osmocom.org/c/osmo-bts/+/35550/comment/9cb207ac_c0128af1 PS1, Line 73: osmo_stderr_target->categories[DHO].loglevel = LOGL_DEBUG; So, if accessing the logging internals is a bad idea (as you mentioned in the related patch), then why not just fix the code to use proper API - `log_set_category_filter()`? -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/35550?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: Ie9c546210bc554843b1029afa20965bde1d7f228 Gerrit-Change-Number: 35550 Gerrit-PatchSet: 1 Gerrit-Owner: Hoernchen <ewild(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: Hoernchen <ewild(a)sysmocom.de> Gerrit-Comment-Date: Thu, 11 Jan 2024 17:41:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0