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

update to go1.18, drop go1.18, and replace deprecated syscall.Syscall<number> #283

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

thaJeztah
Copy link
Contributor

gha: update golangci-lint to v1.52.x

update to go1.18

tools/mkwinsyscall: replace deprecated funcs with SyscallN (drops go1.17)

The "syscall.Syscall" functions were deprecated in go1.18 in favor
of the "syscall.SyscallN" function, which does not need the "nargs" argument,
and does not need the list of arguments to be padded with zeros.
https://github.com/golang/go/blob/go1.18/src/syscall/dll_windows.go#L27-L45

Now that go1.17 reached EOL and is no longer maintained, we can update the
code to use the new SyscallN function. This patch updates the mkwinsyscall
utility to generate code using the new SyscallN function, and removes the
utilities that are now redundant.

internal/socket: replace deprecated syscall.Syscall9

r1, _, e1 := syscall.Syscall(procbind.Addr(), 3, uintptr(s), uintptr(name), uintptr(namelen))
r1, _, e1 := syscall.SyscallN(procbind.Addr(), uintptr(s), uintptr(name), uintptr(namelen))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

⚠️ I should add that I patched these manually (don't think I can use go generate unless doing so on a Windows machine).

Not 100% sure if CI here verifies these files (if not, perhaps someone could double-check them and run go generate on a Windows machine)

Copy link
Contributor Author

@thaJeztah thaJeztah Apr 8, 2023

Choose a reason for hiding this comment

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

Not 100% sure if CI here verifies these files (if not, perhaps someone could double-check them and run go generate on a Windows machine)

looks like it does, and CI looks happy; https://github.com/thaJeztah/go-winio/actions/runs/4645965648

@thaJeztah thaJeztah force-pushed the remove_deprecated_syscall branch 6 times, most recently from 1d52ae4 to 7f78199 Compare April 8, 2023 15:56
    wim/lzx/lzx.go:492:3: early-return: if c { ... } else { ... break } can be simplified to if !c { ... break } ... (revive)
            if matchoffset <= i && matchlen <= end-i {
                copyend := i + matchlen
                for ; i < copyend; i++ {
                    f.window[i] = f.window[i-matchoffset]
                }
            } else {
                f.fail(errCorrupt)
                break
            }

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
pkg/security:

    Error: directive `//nolint:structcheck,unused // structcheck thinks fields are unused, but the are used to pass data to OS` is unused for linter "unused" (nolintlint)

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
pkg/etw, pkg/etwlogrus: replace interface{} with any (revive)

    Warning: use-any: since GO 1.18 'interface{}' can be replaced by 'any' (revive)
    Warning: use-any: since GO 1.18 'interface{}' can be replaced by 'any' (revive)

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
….17)

The "syscall.Syscall<number>" functions were deprecated in go1.18 in favor
of the "syscall.SyscallN" function, which does not need the "nargs" argument,
and does not need the list of arguments to be padded with zeros.
https://github.com/golang/go/blob/go1.18/src/syscall/dll_windows.go#L27-L45

Now that go1.17 reached EOL and is no longer maintained, we can update the
code to use the new SyscallN function. This patch updates the mkwinsyscall
utility to generate code using the new SyscallN function, and removes the
utilities that are now redundant.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah marked this pull request as ready for review April 8, 2023 16:26
@thaJeztah thaJeztah requested a review from a team as a code owner April 8, 2023 16:26
@thaJeztah
Copy link
Contributor Author

This should be ready for review /cc @kevpar 😄

(I should probably point out that containerd 1.6 still tests against go1.17 as "old version", in case this version from main ends up in containerd at some point; https://github.com/containerd/containerd/blob/v1.6.20/.github/workflows/ci.yml#L236)

@helsaawy helsaawy mentioned this pull request Apr 1, 2024
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

1 participant