🐛 Fix implicit aliasing issue in nested maps #810
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
the
size/XS
shyamradhakrishnan
force-pushed
the
shyamradhakrishnan-fix-809
branch
from
a12cc5c
to
a48b995
Compare
k8s-ci-robot
added
size/M
shyamradhakrishnan
changed the title
| @@ -374,7 +374,8 @@ func (c *copyMethodMaker) genMapDeepCopy(actualName *namingInfo, mapType *types. | |||
| c.IfElse("val == nil", func() { | |||
| c.Line("(*out)[key] = nil") | |||
| }, func() { | |||
| c.Line("in, out := &val, &outVal") | |||
| c.Line("inVal := (*in)[key]") | |||
| c.Line("in, out := &inVal, &outVal") | |||
There was a problem hiding this comment.
Thank you a lot for the contribution 🥇
Could you please ensure that the change/issue is also covered with a test?
There was a problem hiding this comment.
Thanks for the review @camilamacedo86 I have added a nested map in cronjob_types.go and that fields and other fields are calling this new line. I hope that will cover as a test case. Can you suggest if I need to add test cases in other places as well?
shyamradhakrishnan
force-pushed
the
shyamradhakrishnan-fix-809
branch
from
a48b995
to
766a447
Compare
|
@vincepri @joelanford @camilamacedo86 gentle reminder for review |
|
Hi, just ran into this as well. Looks fairly complete to me and 2 months passed, so maybe a gentle nudge is fair by now. |
|
@vincepri @joelanford @camilamacedo86 gentle reminder for review |
|
Played a bit around with it. Works as expected /lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sbueringer, shyamradhakrishnan The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
Two notable changes occurred with the generated
deep copy code:
1. Removed obsolete build tags
- kubernetes-sigs/controller-tools#828
2. Fix implicit for loop aliasing
- kubernetes-sigs/controller-tools#810
Signed-off-by: Lance Austin <[email protected]>
Two notable changes occurred with the generated
deep copy code:
1. Removed obsolete build tags
- kubernetes-sigs/controller-tools#828
2. Fix implicit for loop aliasing
- kubernetes-sigs/controller-tools#810
Signed-off-by: Lance Austin <[email protected]>
Fix #809
The PR fixes the implicit memory aliasing issue in autogenerated DeepCopy method when the field is a nested map.