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: 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. 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. 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>