A bug that mutates a not-used copy of a struct

[guodongli-google]

What happened:

A static analyzer DeepGo reports a finding that reveals a coding issue.

What you expected to happen:

At https://github.com/kubernetes/kubernetes/blob/master/vendor/github.com/emicklei/go-restful/route.go#L168-L169

Here Route is struct which will be copied by value. The mutation on the value receiver r doesn't do anything to the original struct in the caller. As a consequence, this mutation has no meaningful effect.

type Route struct {
	Method   string
	...
	contentEncodingEnabled *bool
}

func (r Route) EnableContentEncoding(enabled bool) {
	r.contentEncodingEnabled = &enabled
}

One fix is to use pointer receiver.

func (r *Route) EnableContentEncoding(enabled bool) {
	r.contentEncodingEnabled = &enabled
}

Another possible fix is to write back enabled into the caller'scontentEncodingEnabled :

func (r Route) EnableContentEncoding(enabled bool) {
	*r.contentEncodingEnabled = enabled
}

How to reproduce it (as minimally and precisely as possible):

A simplified bug reproducer is available at: https://play.golang.org/p/cibNCyIarZS

Anything else we need to know?:

Related issue: #105155

Found by #deepgo.

[shivanshuraj1333]

/triage accepted
/area e2e-test-framework
/sig testing

[shivanshuraj1333]

/remove-sig testing

[k8s-ci-robot]

@guodongli-google: There are no sig labels on this issue. Please add an appropriate label by using one of the following commands:

• /sig <group-name>
• /wg <group-name>
• /committee <group-name>

Please see the group list for a listing of the SIGs, working groups, and committees available.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[shivanshuraj1333]

/assign

[shivanshuraj1333]

Filled the bug in the vendor's repository, we can close this now @guodongli-google

[neolit123]

looks like EnableContentEncoding() is not used by k/k code too.

/close

[k8s-ci-robot]

@neolit123: Closing this issue.

In response to this:

looks like EnableContentEncoding() is not used by k/k code too.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.