From 315f5955303cb362c0f55ead6ca3096d3159aef6 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 8 Apr 2025 13:05:48 -0700 Subject: [PATCH 1/5] fs2: refactor numToStr 1. Use tagged switch on value, fixing the staticcheck linter warning: > fs2/memory.go:22:2: QF1002: could use tagged switch on value (staticcheck) > switch { > ^ 2. Eliminate named return, and return early, simplifying the code. Signed-off-by: Kir Kolyshkin --- fs2/memory.go | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/fs2/memory.go b/fs2/memory.go index d67fd8a..f2dc4e1 100644 --- a/fs2/memory.go +++ b/fs2/memory.go @@ -18,17 +18,14 @@ import ( // cgroupv2 files with .min, .max, .low, or .high suffix. // The value of -1 is converted to "max" for cgroupv1 compatibility // (which used to write -1 to remove the limit). -func numToStr(value int64) (ret string) { - switch { - case value == 0: - ret = "" - case value == -1: - ret = "max" - default: - ret = strconv.FormatInt(value, 10) - } - - return ret +func numToStr(value int64) string { + switch value { + case 0: + return "" + case -1: + return "max" + } + return strconv.FormatInt(value, 10) } func isMemorySet(r *cgroups.Resources) bool { From 6136f41398c1aa6fffec339c46c47614297782b3 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 8 Apr 2025 13:17:00 -0700 Subject: [PATCH 2/5] fs2: ignore staticcheck QF1001 De Morgan law warning Ignore this: > fs2/memory.go:57:7: QF1001: could apply De Morgan's law (staticcheck) > if !(errors.Is(err, os.ErrNotExist) && (swapStr == "max" || swapStr == "0")) { > ^ Because it its current form, the if condition corresponds to the preceding comment. Signed-off-by: Kir Kolyshkin --- fs2/memory.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs2/memory.go b/fs2/memory.go index f2dc4e1..7613307 100644 --- a/fs2/memory.go +++ b/fs2/memory.go @@ -54,7 +54,7 @@ func setMemory(dirPath string, r *cgroups.Resources) error { if swapStr != "" { if err := cgroups.WriteFile(dirPath, "memory.swap.max", swapStr); err != nil { // If swap is not enabled, silently ignore setting to max or disabling it. - if !(errors.Is(err, os.ErrNotExist) && (swapStr == "max" || swapStr == "0")) { + if !(errors.Is(err, os.ErrNotExist) && (swapStr == "max" || swapStr == "0")) { //nolint:staticcheck // Ignore "QF1001: could apply De Morgan's law". return err } } From f02578ce4c1a0865a094e9924e5bf437ae274138 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 8 Apr 2025 13:24:02 -0700 Subject: [PATCH 3/5] devices: fix some comments Since commit 8a90b8b the emulator is private, but it is still referred to as Emulator. Fix this. Signed-off-by: Kir Kolyshkin --- devices/devices_emulator.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/devices/devices_emulator.go b/devices/devices_emulator.go index ab18268..890d36f 100644 --- a/devices/devices_emulator.go +++ b/devices/devices_emulator.go @@ -261,7 +261,7 @@ func (e *emulator) Apply(rule devices.Rule) error { } // emulatorFromList takes a reader to a "devices.list"-like source, and returns -// a new Emulator that represents the state of the devices cgroup. Note that +// a new emulator that represents the state of the devices cgroup. Note that // black-list devices cgroups cannot be fully reconstructed, due to limitations // in the devices cgroup API. Instead, such cgroups are always treated as // "allow all" cgroups. @@ -301,7 +301,7 @@ func emulatorFromList(list io.Reader) (*emulator, error) { // disruptive rules (like denying all device access) will only be applied if // necessary. // -// This function is the sole reason for all of Emulator -- to allow us +// This function is the sole reason for all of emulator -- to allow us // to figure out how to update a containers' cgroups without causing spurious // device errors (if possible). func (source *emulator) Transition(target *emulator) ([]*devices.Rule, error) { //nolint:revive // Ignore receiver-naming warning. From b66a3ea511a71f6454eb281177cb3b251679f638 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 8 Apr 2025 13:24:58 -0700 Subject: [PATCH 4/5] devices: rm nolint annotations Both revive and staticcheck complains like this: > devices/devices_emulator.go:239:20: ST1016: methods on the same type should have the same receiver name (seen 1x "source", 8x "e") (staticcheck) > func (e *emulator) Apply(rule devices.Rule) error { > ^ What they mean to point to is this line: > func (source *emulator) Transition(target *emulator) ([]*devices.Rule, error) { In here, "source" does actually make sense, but instead of suppressing these warnings, let's just give up and rename the receiver, and introduce a source variable instead. Signed-off-by: Kir Kolyshkin --- devices/devices_emulator.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/devices/devices_emulator.go b/devices/devices_emulator.go index 890d36f..df258fb 100644 --- a/devices/devices_emulator.go +++ b/devices/devices_emulator.go @@ -304,8 +304,9 @@ func emulatorFromList(list io.Reader) (*emulator, error) { // This function is the sole reason for all of emulator -- to allow us // to figure out how to update a containers' cgroups without causing spurious // device errors (if possible). -func (source *emulator) Transition(target *emulator) ([]*devices.Rule, error) { //nolint:revive // Ignore receiver-naming warning. +func (e *emulator) Transition(target *emulator) ([]*devices.Rule, error) { var transitionRules []*devices.Rule + source := e oldRules := source.rules // If the default policy doesn't match, we need to include a "disruptive" From 5506094eefc5888fa92a433f19d48ad10f05bacf Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 8 Apr 2025 13:34:02 -0700 Subject: [PATCH 5/5] ci: switch to golangci-lint v2 The new configs were initially created by golangci-lint migrate, then tailored to simplify and minify. Signed-off-by: Kir Kolyshkin --- .github/workflows/validate.yml | 4 ++-- .golangci-extra.yml | 15 ++++++++++++--- .golangci.yml | 29 +++++++++++++++++++++++------ 3 files changed, 37 insertions(+), 11 deletions(-) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 8c28fb9..4517406 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -27,9 +27,9 @@ jobs: - uses: actions/setup-go@v5 with: go-version: "${{ env.GO_VERSION }}" - - uses: golangci/golangci-lint-action@v6 + - uses: golangci/golangci-lint-action@v7 with: - version: v1.64 + version: v2.0 # Extra linters, only checking new code from a pull request. - name: lint-extra if: github.event_name == 'pull_request' diff --git a/.golangci-extra.yml b/.golangci-extra.yml index 1a12d7b..b98dba1 100644 --- a/.golangci-extra.yml +++ b/.golangci-extra.yml @@ -2,11 +2,20 @@ # github PRs only (see lint-extra in .github/workflows/validate.yml). # # For the default linter config, see .golangci.yml. This config should -# only enable additional linters not enabled in the default config. +# only enable additional linters and/or linter settings not enabled +# in the default config. +version: "2" linters: - disable-all: true + default: none enable: - godot - revive - + - staticcheck + settings: + staticcheck: + checks: + - all + - -QF1008 # https://staticcheck.dev/docs/checks/#QF1008 Omit embedded fields from selector expression. + exclusions: + generated: strict diff --git a/.golangci.yml b/.golangci.yml index 197671b..9079988 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,14 +1,31 @@ # For documentation, see https://golangci-lint.run/usage/configuration/ +version: "2" + +formatters: + enable: + - gofumpt + exclusions: + generated: strict linters: enable: - errorlint - - gofumpt - nolintlint - unconvert - unparam - -linters-settings: - govet: - enable: - - nilness + settings: + govet: + enable: + - nilness + staticcheck: + checks: + - all + - -ST1000 # https://staticcheck.dev/docs/checks/#ST1000 Incorrect or missing package comment. + - -ST1003 # https://staticcheck.dev/docs/checks/#ST1003 Poorly chosen identifier. + - -ST1005 # https://staticcheck.dev/docs/checks/#ST1005 Incorrectly formatted error string. + - -QF1008 # https://staticcheck.dev/docs/checks/#QF1008 Omit embedded fields from selector expression. + exclusions: + generated: strict + presets: + - comments + - std-error-handling