Skip to content

Nits#2325

Merged
AkihiroSuda merged 2 commits into
opencontainers:masterfrom
kolyshkin:nits-2
Apr 19, 2020
Merged

Nits#2325
AkihiroSuda merged 2 commits into
opencontainers:masterfrom
kolyshkin:nits-2

Conversation

@kolyshkin
Copy link
Copy Markdown
Contributor

A few nitpicks to simplify the code.

1. Do not use syscall package where unix can be used.

In many places (not all of them though) we can use unix.
instead of syscall. as these are indentical.

In particular, x/sys/unix defines:

type Signal = syscall.Signal
type Errno = syscall.Errno
type SysProcAttr = syscall.SysProcAttr

const ENODEV      = syscall.Errno(0x13)

and unix.Exec() calls syscall.Exec().

2. Use errors.Is() to check for EINTR.

This is a forgotten hunk from PR #2291.

In many places (not all of them though) we can use `unix.`
instead of `syscall.` as these are indentical.

In particular, x/sys/unix defines:

```go
type Signal = syscall.Signal
type Errno = syscall.Errno
type SysProcAttr = syscall.SysProcAttr

const ENODEV      = syscall.Errno(0x13)
```

and unix.Exec() calls syscall.Exec().

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This is a forgotten hunk from PR opencontainers#2291.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Comment thread kill.go
for {
err := ioutil.WriteFile(filename, data, perm)
if isInterruptedWriteFile(err) {
if errors.Is(err, unix.EINTR) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Tested using go 1.14.2:

=== RUN   TestWriteCgroupFileHandlesInterrupt
INFO[0000] interrupted while writing 1112067 to /sys/fs/cgroup/memory/test-eint-651408428/memory.limit_in_bytes 
--- PASS: TestWriteCgroupFileHandlesInterrupt (0.75s)
=== RUN   TestGetCgroupParamsInt
--- PASS: TestGetCgroupParamsInt (0.00s)
PASS

@cyphar
Copy link
Copy Markdown
Member

cyphar commented Apr 19, 2020

LGTM.

Approved with PullApprove

@AkihiroSuda
Copy link
Copy Markdown
Member

AkihiroSuda commented Apr 19, 2020

LGTM

Approved with PullApprove

@AkihiroSuda AkihiroSuda merged commit cd5f4fd into opencontainers:master Apr 19, 2020
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.

3 participants