kubeadmiral icon indicating copy to clipboard operation
kubeadmiral copied to clipboard

Should not `append` to global slices

Open gary-lgy opened this issue 1 year ago • 1 comments

image

Sometimes we append to these global slices. If a slice has cap > len, then appending to it would mutate the underlying storage. ​https://go.dev/play/p/f5YRITlfiGj. (Golang doesn't seem to guarantee that []int{2,3} must have a capacity of 2.)

If we append to these global slices on different goroutines, it would result in a data race.

gary-lgy avatar Apr 25 '23 09:04 gary-lgy

[]int{2, 3} is explicitly defined to have a capacity of 2. From the Go language specification:

A slice literal describes the entire underlying array literal. Thus the length and capacity of a slice literal are the maximum element index plus one.

Furthermore, append is defined to allocate a copy and returns it in the specification:

If the capacity of s is not large enough to fit the additional values, append allocates a new, sufficiently large underlying array that fits both the existing slice elements and the additional values. Otherwise, append re-uses the underlying array.

Therefore, it is actually specified behavior that append cannot cause problems with string literals.

That said, it is dangerous to rely on the magical behavior of append, e.g. if someone somehow changes the code to append(TemplatePath[:1], PlacementsField), the global variable would indeed get written, so I agree that it is a good idea to avoid the use of global variables at all.

SOF3 avatar Apr 25 '23 10:04 SOF3