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

[L] Change in osmo-msc[master]: Use gsup_client_mux from libosmo-gsup-client

by pespin

Attention is currently required from: daniel, fixeria, lynxis lazus, osmith. Hello Jenkins Builder, daniel, fixeria, lynxis lazus, osmith, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-msc/+/39019?usp=email to look at the new patch set (#3). The following approvals got outdated and were removed: Code-Review+1 by fixeria, Code-Review+1 by osmith, Verified+1 by Jenkins Builder Change subject: Use gsup_client_mux from libosmo-gsup-client ...................................................................... Use gsup_client_mux from libosmo-gsup-client gsup_client_mux code was cloned a couple months ago to libosmo-gsup-client, use it in osmo-msc and drop its own copy. The code was not dropped at the time because it required tweaking the unit tests, because they were wrapping some underlaying functions which cannot be wrapped anymore since the code using them is now inside the separate already-linked libosmo-gsup-client.so. Solve it by actually wrapping one layer up in the gsup_client_mux itself since it now something we mantain here anymore. Change-Id: I34d02ae219f3ec8daf47942c46f52b21eab21242 --- M TODO-RELEASE M include/osmocom/msc/Makefile.am D include/osmocom/msc/gsup_client_mux.h M src/libmsc/Makefile.am M src/libmsc/e_link.c M src/libmsc/gsm_04_11_gsup.c M src/libmsc/gsm_09_11.c D src/libmsc/gsup_client_mux.c M src/libmsc/msc_net_init.c M src/libvlr/vlr.c M src/osmo-msc/msc_main.c M tests/msc_vlr/Makefile.am M tests/msc_vlr/msc_vlr_tests.c 13 files changed, 49 insertions(+), 250 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/19/39019/3 -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/39019?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I34d02ae219f3ec8daf47942c46f52b21eab21242 Gerrit-Change-Number: 39019 Gerrit-PatchSet: 3 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: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu>

6 months, 3 weeks

1
0
0 0

[L] Change in osmo-msc[master]: Use gsup_client_mux from libosmo-gsup-client

by pespin

Attention is currently required from: daniel, lynxis lazus, osmith. pespin has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-msc/+/39019?usp=email ) Change subject: Use gsup_client_mux from libosmo-gsup-client ...................................................................... Patch Set 2: (1 comment) File tests/msc_vlr/msc_vlr_tests.c: https://gerrit.osmocom.org/c/osmo-msc/+/39019/comment/e6539e55_a6469a68?usp… : PS2, Line 843: /* override, requires '-Wl,--wrap=gsup_client_send' */ > or rather just `--wrap=gsup_client_mux_tx` here. […] Done -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/39019?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I34d02ae219f3ec8daf47942c46f52b21eab21242 Gerrit-Change-Number: 39019 Gerrit-PatchSet: 2 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: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Comment-Date: Wed, 04 Dec 2024 13:44:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: osmith <osmith(a)sysmocom.de>

6 months, 3 weeks

1
0
0 0

[XS] Change in osmo-hlr[master]: gsup_client: Avoid double memset 0

by pespin

Attention is currently required from: fixeria, laforge, osmith. pespin has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-hlr/+/39006?usp=email ) Change subject: gsup_client: Avoid double memset 0 ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: Oh nice, now with the new suggested approach tests are failing here. You know what, I think I'll simply abandon this patch, I just wanted to submit a quick improvement while reading the code. -- To view, visit https://gerrit.osmocom.org/c/osmo-hlr/+/39006?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Change-Id: I66515fc893ffc64a36abedc01a75b57aed7e932d Gerrit-Change-Number: 39006 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Wed, 04 Dec 2024 13:37:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

6 months, 3 weeks

1
0
0 0

[M] Change in osmo-hlr[master]: gsup_client: Add new APIs to avoid users accessing struct fields

by pespin

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-hlr/+/39009?usp=email ) ( 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: gsup_client: Add new APIs to avoid users accessing struct fields ...................................................................... gsup_client: Add new APIs to avoid users accessing struct fields This is a first step towards changing gsup_client implementation to use an osmo_stream_cli instead of libosmo-abis' ipa_client_conn. The libosmo-abis' ipa_client_conn will eventually be deprecated together with all IPA related code in libosmo-abis. In order to be able to make the implementation change, we first need to make sure no users are using the struct fields of gsup_client (this patch); 2nd step will be making the struct private by moving it to a private header; 3rd step will be changing the implementation to use osmo_stream. Related: OS#5896 Change-Id: I401af83232022f1c141eef1f428cbe206a8aaaa2 --- M TODO-RELEASE M include/osmocom/gsupclient/gsup_client.h M src/gsupclient/gsup_client.c M src/gsupclient/gsup_client_mux.c M src/remote_hlr.c 5 files changed, 59 insertions(+), 8 deletions(-) Approvals: laforge: Looks good to me, but someone else must approve osmith: Looks good to me, approved fixeria: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/TODO-RELEASE b/TODO-RELEASE index 0ed7189..f14e8b5 100644 --- a/TODO-RELEASE +++ b/TODO-RELEASE @@ -7,3 +7,6 @@ # If any interfaces have been added since the last public release: c:r:a + 1. # If any interfaces have been removed or changed since the last public release: c:r:0. #library what description / commit summary line +libosmo-gsup-client new API osmo_gsup_client_{get,set}_data(), + osmo_gsup_client_get_rem_addr(), osmo_gsup_client_get_rem_port(), + osmo_gsup_client_is_connected(), osmo_gsup_client_get_ipaccess_unit() \ No newline at end of file diff --git a/include/osmocom/gsupclient/gsup_client.h b/include/osmocom/gsupclient/gsup_client.h index ea66ca1..61cd45d 100644 --- a/include/osmocom/gsupclient/gsup_client.h +++ b/include/osmocom/gsupclient/gsup_client.h @@ -20,6 +20,9 @@ */ #pragma once +#include <stdint.h> +#include <stdbool.h> + #include <osmocom/core/timer.h> #include <osmocom/gsm/oap_client.h> #include <osmocom/gsm/ipa.h> @@ -40,6 +43,7 @@ typedef bool (*osmo_gsup_client_up_down_cb_t)(struct osmo_gsup_client *gsupc, bool up); +/* NOTE: THIS STRUCT IS CONSIDERED PRIVATE, AVOID ACCESSING ITS FIELDS! */ struct osmo_gsup_client { const char *unit_name; /* same as ipa_dev->unit_name, for backwards compat */ @@ -99,3 +103,12 @@ const struct osmo_gsup_message *gsup_msg); struct msgb *osmo_gsup_client_msgb_alloc(void); +void *osmo_gsup_client_get_data(const struct osmo_gsup_client *gsupc); +void osmo_gsup_client_set_data(struct osmo_gsup_client *gsupc, void *data); + +const char *osmo_gsup_client_get_rem_addr(const struct osmo_gsup_client *gsupc); +uint16_t osmo_gsup_client_get_rem_port(const struct osmo_gsup_client *gsupc); + +bool osmo_gsup_client_is_connected(const struct osmo_gsup_client *gsupc); +const struct ipaccess_unit *osmo_gsup_client_get_ipaccess_unit(const struct osmo_gsup_client *gsupc); + diff --git a/src/gsupclient/gsup_client.c b/src/gsupclient/gsup_client.c index e7d03f2..1168c9d 100644 --- a/src/gsupclient/gsup_client.c +++ b/src/gsupclient/gsup_client.c @@ -450,3 +450,36 @@ { return msgb_alloc_headroom(4000, 64, __func__); } + +void *osmo_gsup_client_get_data(const struct osmo_gsup_client *gsupc) +{ + return gsupc->data; +} + +void osmo_gsup_client_set_data(struct osmo_gsup_client *gsupc, void *data) +{ + gsupc->data = data; +} + +const char *osmo_gsup_client_get_rem_addr(const struct osmo_gsup_client *gsupc) +{ + if (!gsupc->link) + return NULL; + return gsupc->link->addr; +} +uint16_t osmo_gsup_client_get_rem_port(const struct osmo_gsup_client *gsupc) +{ + if (!gsupc->link) + return 0; + return gsupc->link->port; +} + +bool osmo_gsup_client_is_connected(const struct osmo_gsup_client *gsupc) +{ + return gsupc->is_connected; +} + +const struct ipaccess_unit *osmo_gsup_client_get_ipaccess_unit(const struct osmo_gsup_client *gsupc) +{ + return gsupc->ipa_dev; +} diff --git a/src/gsupclient/gsup_client_mux.c b/src/gsupclient/gsup_client_mux.c index 7699ca1..45ebd75 100644 --- a/src/gsupclient/gsup_client_mux.c +++ b/src/gsupclient/gsup_client_mux.c @@ -58,7 +58,7 @@ /* Non-static for unit tests */ int gsup_client_mux_rx(struct osmo_gsup_client *gsup_client, struct msgb *msg) { - struct gsup_client_mux *gcm = gsup_client->data; + struct gsup_client_mux *gcm = osmo_gsup_client_get_data(gsup_client); struct osmo_gsup_message gsup; enum osmo_gsup_message_class message_class; int rc; @@ -115,7 +115,7 @@ &gsup_client_mux_rx, NULL); if (!gcm->gsup_client) return -ENOMEM; - gcm->gsup_client->data = gcm; + osmo_gsup_client_set_data(gcm->gsup_client, gcm); return 0; } @@ -144,14 +144,16 @@ struct osmo_gsup_message *gsup_msg) { const char *local_msc_name; + const struct ipaccess_unit *ipa_dev; if (!gcm) return; if (!gcm->gsup_client) return; - if (!gcm->gsup_client->ipa_dev) + ipa_dev = osmo_gsup_client_get_ipaccess_unit(gcm->gsup_client); + if (!ipa_dev) return; - local_msc_name = gcm->gsup_client->ipa_dev->serno; + local_msc_name = ipa_dev->serno; if (!local_msc_name) return; gsup_msg->source_name = (const uint8_t *) local_msc_name; diff --git a/src/remote_hlr.c b/src/remote_hlr.c index 00bfbb1..93157a3 100644 --- a/src/remote_hlr.c +++ b/src/remote_hlr.c @@ -66,7 +66,7 @@ * The local MSC shall be indicated by gsup.destination_name. */ static int remote_hlr_rx(struct osmo_gsup_client *gsupc, struct msgb *msg) { - struct remote_hlr *rh = gsupc->data; + struct remote_hlr *rh = osmo_gsup_client_get_data(gsupc); struct proxy_subscr proxy_subscr; struct osmo_gsup_message gsup; int rc; @@ -108,7 +108,7 @@ static bool remote_hlr_up_down(struct osmo_gsup_client *gsupc, bool up) { - struct remote_hlr *remote_hlr = gsupc->data; + struct remote_hlr *remote_hlr = osmo_gsup_client_get_data(gsupc); struct remote_hlr_pending_up *p, *n; if (!up) { LOG_REMOTE_HLR(remote_hlr, LOGL_NOTICE, "link to remote HLR is down, removing GSUP client\n"); @@ -127,7 +127,7 @@ bool remote_hlr_is_up(struct remote_hlr *remote_hlr) { - return remote_hlr && remote_hlr->gsupc && remote_hlr->gsupc->is_connected; + return remote_hlr && remote_hlr->gsupc && osmo_gsup_client_is_connected(remote_hlr->gsupc); } struct remote_hlr *remote_hlr_get_or_connect(const struct osmo_sockaddr_str *addr, bool connect, @@ -180,7 +180,7 @@ return NULL; } - rh->gsupc->data = rh; + osmo_gsup_client_set_data(rh->gsupc, rh); llist_add(&rh->entry, &remote_hlrs); add_result_cb: -- To view, visit https://gerrit.osmocom.org/c/osmo-hlr/+/39009?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Change-Id: I401af83232022f1c141eef1f428cbe206a8aaaa2 Gerrit-Change-Number: 39009 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

6 months, 3 weeks

1
0
0 0

[XS] Change in osmo-hlr[master]: gsup_client: Avoid double memset 0

by pespin

Attention is currently required from: fixeria, laforge, osmith. Hello Jenkins Builder, fixeria, laforge, osmith, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-hlr/+/39006?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Code-Review+1 by laforge, Code-Review+2 by osmith, Code-Review-1 by fixeria, Verified+1 by Jenkins Builder The change is no longer submittable: Code-Review and Verified are unsatisfied now. Change subject: gsup_client: Avoid double memset 0 ...................................................................... gsup_client: Avoid double memset 0 Memory is already zeroed during talloc_zero, no need to reset it to zero. Change-Id: I66515fc893ffc64a36abedc01a75b57aed7e932d --- M src/gsupclient/gsup_client.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/06/39006/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-hlr/+/39006?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Change-Id: I66515fc893ffc64a36abedc01a75b57aed7e932d Gerrit-Change-Number: 39006 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>

6 months, 3 weeks

1
0
0 0

[M] Change in osmo-hlr[master]: gsup_client: Add new APIs to avoid users accessing struct fields

by pespin

Attention is currently required from: pespin. Hello Jenkins Builder, fixeria, laforge, osmith, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-hlr/+/39009?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Verified+1 by Jenkins Builder The change is no longer submittable: Verified is unsatisfied now. Change subject: gsup_client: Add new APIs to avoid users accessing struct fields ...................................................................... gsup_client: Add new APIs to avoid users accessing struct fields This is a first step towards changing gsup_client implementation to use an osmo_stream_cli instead of libosmo-abis' ipa_client_conn. The libosmo-abis' ipa_client_conn will eventually be deprecated together with all IPA related code in libosmo-abis. In order to be able to make the implementation change, we first need to make sure no users are using the struct fields of gsup_client (this patch); 2nd step will be making the struct private by moving it to a private header; 3rd step will be changing the implementation to use osmo_stream. Related: OS#5896 Change-Id: I401af83232022f1c141eef1f428cbe206a8aaaa2 --- M TODO-RELEASE M include/osmocom/gsupclient/gsup_client.h M src/gsupclient/gsup_client.c M src/gsupclient/gsup_client_mux.c M src/remote_hlr.c 5 files changed, 59 insertions(+), 8 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/09/39009/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-hlr/+/39009?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Change-Id: I401af83232022f1c141eef1f428cbe206a8aaaa2 Gerrit-Change-Number: 39009 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de>

6 months, 3 weeks

1
0
0 0

[S] Change in osmo-hlr[master]: gsup_client: Avoid double memset 0

by pespin

Attention is currently required from: fixeria. pespin has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-hlr/+/39006?usp=email ) Change subject: gsup_client: Avoid double memset 0 ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > I don't really agree with half of what you say there, but it makes no sense to keep discussing I'll […] Done -- To view, visit https://gerrit.osmocom.org/c/osmo-hlr/+/39006?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Change-Id: I66515fc893ffc64a36abedc01a75b57aed7e932d Gerrit-Change-Number: 39006 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Wed, 04 Dec 2024 13:27:32 +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>

6 months, 3 weeks

1
0
0 0

[S] Change in osmo-hlr[master]: gsup_client: Avoid double memset 0

by pespin

Attention is currently required from: fixeria. pespin has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-hlr/+/39006?usp=email ) Change subject: gsup_client: Avoid double memset 0 ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > > When I mean "all memory" I mean "all meaningful memory not consisting of padding". […] I don't really agree with half of what you say there, but it makes no sense to keep discussing I'll just fix it. I do agree that using talloc() + setting fields may be a bit more efficient but I don't really care here tbh. -- To view, visit https://gerrit.osmocom.org/c/osmo-hlr/+/39006?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Change-Id: I66515fc893ffc64a36abedc01a75b57aed7e932d Gerrit-Change-Number: 39006 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Wed, 04 Dec 2024 13:24:38 +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>

6 months, 3 weeks

1
0
0 0

[S] Change in osmo-hlr[master]: gsup_client: Avoid double memset 0

by fixeria

Attention is currently required from: pespin. fixeria has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-hlr/+/39006?usp=email ) Change subject: gsup_client: Avoid double memset 0 ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > When I mean "all memory" I mean "all meaningful memory not consisting of padding". Well, your commit message says "double memset", while this is clearly not an equivalent of `memset()`. There's only one `memset()` both before and after your change. > This doesn't change anyway the fact that memory in fields is set twice. It does. If you change from `talloc_zero()` to `talloc()`, then you would no longer be setting fields twice. The struct assignment (as it's done now) takes care of that. > TBH I don't see why are you setting a -1 here, it makes no sense [...] I added CR-1 because I disagree with this change, even though it does not change the logic. I am pretty sure the author of this code would not like such a change either, but sadly he's not actively doing code review... My point is that a) yes, current code is not perfect and assigning fields twice (first to 0 and then to actual values), but b) struct assignment is not equivalent to `memset()` and there's nothing wrong with using it. By changing from `talloc_zero()` to `talloc()` you would actually fix double assignment. -- To view, visit https://gerrit.osmocom.org/c/osmo-hlr/+/39006?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Change-Id: I66515fc893ffc64a36abedc01a75b57aed7e932d Gerrit-Change-Number: 39006 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Wed, 04 Dec 2024 13:17:12 +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>

6 months, 3 weeks

1
0
0 0

[M] Change in osmo-remsim[master]: Remove rspro_client.c which is not used anyway

by pespin

Attention is currently required from: laforge, pespin. Hello Jenkins Builder, laforge, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-remsim/+/39021?usp=email to look at the new patch set (#3). The following approvals got outdated and were removed: Code-Review-1 by laforge, Verified+1 by Jenkins Builder Change subject: Remove rspro_client.c which is not used anyway ...................................................................... Remove rspro_client.c which is not used anyway rspro_client.c was already removed ~6 years ago in 511c51313d4b1994eaa5faebcf01e8e24fb8b5a5, so those APIs where not really implemented and hence used. Remove the header. Related: OS#5896 Change-Id: I3bacae853101a79804553175ebd4482acb188597 --- M include/osmocom/rspro/Makefile.am D include/osmocom/rspro/rspro_client.h 2 files changed, 0 insertions(+), 64 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-remsim refs/changes/21/39021/3 -- To view, visit https://gerrit.osmocom.org/c/osmo-remsim/+/39021?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: osmo-remsim Gerrit-Branch: master Gerrit-Change-Id: I3bacae853101a79804553175ebd4482acb188597 Gerrit-Change-Number: 39021 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de>

6 months, 3 weeks

1
0
0 0

2025

2024

2023

2022

gerrit-log December 2024