Attention is currently required from: pespin.
1 comment:
Patchset:
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 change 39006. To unsubscribe, or for help writing mail filters, visit settings.