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.