Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issues found by linter #37

Merged
merged 18 commits into from Aug 28, 2023

Conversation

p-strusiewiczsurmacki-mobica
Copy link
Contributor

This PR is continuation of #36
It fixes all the issues found by the golangci-lint configuration introduced in #36.

NOTE: This PR should be fully tested in production-grade test environment before being merged, as I am not able to do so.

Copy link
Member

@schrej schrej left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job!

A few nits, but looks good overall.

I'll leave it to @chdxD1 to test this and to consider the two fixed allocation bugs.

pkg/anycast/anycast.go Outdated Show resolved Hide resolved
pkg/anycast/anycast.go Show resolved Hide resolved
pkg/bpf/loader.go Outdated Show resolved Hide resolved
pkg/frr/configure.go Outdated Show resolved Hide resolved
pkg/frr/configure.go Outdated Show resolved Hide resolved
pkg/reconciler/reconciler.go Outdated Show resolved Hide resolved
pkg/reconciler/layer2.go Outdated Show resolved Hide resolved
pkg/reconciler/layer2.go Outdated Show resolved Hide resolved
if addr.Scope != unix.RT_SCOPE_UNIVERSE {
continue
}
info.AnycastGateways = append(info.AnycastGateways, &addr)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chdxD1 this was a bug that is getting fixed in this PR.
By simply referencing it this way, the same address is added multiple times: https://husni.dev/beware-of-implicit-memory-aliasing-in-go-foor-loop/

if addr.Scope != unix.RT_SCOPE_UNIVERSE {
continue
}
info.AnycastGateways = append(info.AnycastGateways, &addr)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chdxD1 same here.

Co-authored-by: Jakob Schrettenbrunner <dev@schrej.net>
Co-authored-by: Jakob Schrettenbrunner <dev@schrej.net>
Co-authored-by: Jakob Schrettenbrunner <dev@schrej.net>
Co-authored-by: Jakob Schrettenbrunner <dev@schrej.net>
@schrej schrej assigned chdxD1 and unassigned chdxD1 Aug 22, 2023
@schrej schrej merged commit 552ebf0 into telekom:main Aug 28, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants