ronantakizawa/codereview-bench huggingface dataset:
Repository: opencontainers/runc
File: libcontainer/cgroups/fs/cpuset.go (Go)

Code before:
package fs

import (
	"errors"
	"os"
	"path/filepath"
	"strconv"
	"strings"
	"sync"

	"golang.org/x/sys/unix"

	"github.com/opencontainers/runc/libcontainer/cgroups"
	"github.com/opencontainers/runc/libcontainer/cgroups/fscommon"
	"github.com/opencontainers/runc/libcontainer/configs"
)

var (
	cpusetLock     sync.Mutex
	cpusetNoPrefix bool
	cpusetFastPath bool
)

func getCpusetFile(path string, name string) string {
	cpusetLock.Lock()
	defer cpusetLock.Unlock()

	// Only the v1 cpuset cgroup is allowed to mount with noprefix.
	// See kernel source: https://github.com/torvalds/linux/blob/2e1b3cc9d7f790145a80cb705b168f05dab65df2/kernel/cgroup/cgroup-v1.c#L1070
	// Cpuset cannot be mounted with and without prefix simultaneously.
	// Commonly used in Android environments.

	if !cpusetFastPath {
		err := unix.Access(filepath.Join(path, "cpuset."+name), unix.F_OK)
		if err == nil {
			// Use the fast path only if we can access one type of mount for cpuset already
			cpusetFastPath = true
		} else {
			err = unix.Access(filepath.Join(path, name), unix.F_OK)
			if err == nil {
				cpusetNoPrefix = true
				cpusetFastPath = true
			}
		}
	}

	if cpusetNoPrefix {
		return name
	}

	return "cpuset." + name

Review comment (suggestion):
Makes sense to add
```go
const prefix="cpuset."
```
and use it below.

Code after:
package fs

import (
	"errors"
	"os"
	"path/filepath"
	"strconv"
	"strings"
	"sync"

	"golang.org/x/sys/unix"

	"github.com/opencontainers/runc/libcontainer/cgroups"
	"github.com/opencontainers/runc/libcontainer/cgroups/fscommon"
)

var (
	cpusetLock     sync.Mutex
	cpusetPrefix   = "cpuset."
	cpusetFastPath bool
)

func cpusetFile(path string, name string) string {
	cpusetLock.Lock()
	defer cpusetLock.Unlock()

	// Only the v1 cpuset cgroup is allowed to mount with noprefix.
	// See kernel source: https://github.com/torvalds/linux/blob/2e1b3cc9d7f790145a80cb705b168f05dab65df2/kernel/cgroup/cgroup-v1.c#L1070
	// Cpuset cannot be mounted with and without prefix simultaneously.
	// Commonly used in Android environments.

	if cpusetFastPath {
		return cpusetPrefix + name
	}

	err := unix.Access(filepath.Join(path, cpusetPrefix+name), unix.F_OK)
	if err == nil {
		// Use the fast path only if we can access one type of mount for cpuset already
		cpusetFastPath = true
	} else {
		err = unix.Access(filepath.Join(path, name), unix.F_OK)
		if err == nil {
			cpusetPrefix = ""
			cpusetFastPath = true
		}
	}

	return cpusetPrefix + name
}

type CpusetGroup struct{}
Repository: opencontainers/runc
File: libcontainer/cgroups/utils_test.go (Go)

Code before:
}

func TestConvertBlkIOToIOWeightValue(t *testing.T) {
	cases := map[uint16]uint64{
		0:    0,
		10:   1,
		1000: 10000,
	}
	for i, expected := range cases {
		got := ConvertBlkIOToIOWeightValue(i)
		if got != expected {
			t.Errorf("expected ConvertBlkIOToIOWeightValue(%d) to be %d, got %d", i, expected, got)
		}
	}
}

// TestRemovePathEROFS is to test remove a non-exist dir in a ro mount point.
// The similar issue example: https://github.com/opencontainers/runc/issues/4518
func TestRemovePathEROFS(t *testing.T) {
	dirFrom, err := os.MkdirTemp(".", "from-")
	if err != nil {
		t.Skip("can't create a tmp dir")
	}
	defer func() {
		_ = unix.Rmdir(dirFrom)
	}()
	dirTo, err := os.MkdirTemp(".", "to-")
	if err != nil {
		t.Skip("can't create a tmp dir")
	}
	defer func() {
		_ = unix.Rmdir(dirTo)
	}()
	err = unix.Mount(dirFrom, dirTo, "", unix.MS_BIND, "")
	if err != nil {
		t.Skip("no permission of mount")
	}
	err = unix.Mount("", dirTo, "", unix.MS_REMOUNT|unix.MS_BIND|unix.MS_RDONLY, "")
	if err != nil {
		t.Skip("no permission of mount")
	}
	defer func() {
		_ = unix.Unmount(dirTo, 0)
	}()
	nonExistDir := filepath.Join(dirTo, "nothing")
	err = rmdir(nonExistDir, true)
	if !errors.Is(err, unix.EROFS) {
		t.Fatalf("expected the error of removing a non-exist dir %s in a ro mount point with rmdir to be unix.EROFS, but got %v", nonExistDir, err)
	}
	err = RemovePath(nonExistDir)
	if err != nil {

Review comment (suggestion):
I _think_ this can be replaced with
```suggestion
	dirFrom := t.TempDir()
	dirTo := t.TempDir()
```

Code after:

func TestConvertBlkIOToIOWeightValue(t *testing.T) {
	cases := map[uint16]uint64{
		0:    0,
		10:   1,
		1000: 10000,
	}
	for i, expected := range cases {
		got := ConvertBlkIOToIOWeightValue(i)
		if got != expected {
			t.Errorf("expected ConvertBlkIOToIOWeightValue(%d) to be %d, got %d", i, expected, got)
		}
	}
}

// TestRemovePathReadOnly is to test remove a non-existent dir in a ro mount point.
// The similar issue example: https://github.com/opencontainers/runc/issues/4518
func TestRemovePathReadOnly(t *testing.T) {
	dirTo := t.TempDir()
	err := unix.Mount(t.TempDir(), dirTo, "", unix.MS_BIND, "")
	if err != nil {
		t.Skip("no permission of mount")
	}
	defer func() {
		_ = unix.Unmount(dirTo, 0)
	}()
	err = unix.Mount("", dirTo, "", unix.MS_REMOUNT|unix.MS_BIND|unix.MS_RDONLY, "")
	if err != nil {
		t.Skip("no permission of mount")
	}
	nonExistentDir := filepath.Join(dirTo, "non-existent-dir")
	err = rmdir(nonExistentDir, true)
	if !errors.Is(err, unix.EROFS) {
		t.Fatalf("expected the error of removing a non-existent dir %s in a ro mount point with rmdir to be unix.EROFS, but got: %v", nonExistentDir, err)
	}
	err = RemovePath(nonExistentDir)
	if err != nil {
		t.Fatalf("expected the error of removing a non-existent dir %s in a ro mount point with RemovePath to be nil, but got: %v", nonExistentDir, err)
	}
}
Repository: opencontainers/runc
File: libcontainer/criu_disabled_linux.go (Go)

Code before:
//go:build runc_nocr

package libcontainer

import "errors"

var ErrNoCR = errors.New("checkpoint/restore support not compiled in")

func (c *Container) Restore(process *Process, criuOpts *CriuOpts) error {
	return ErrNoCR
}

func (c *Container) Checkpoint(criuOpts *CriuOpts) error {
	return ErrNoCR
}


Review comment (suggestion):
Since this is going to be a user-visible error, we should probably make the error a little more friendly, something like:

```suggestion
var ErrNoCR = errors.New("this runc binary has not been compiled with checkpoint/restore support enabled (runc_nocr)")
```

Code after:
//go:build runc_nocriu

package libcontainer

import "errors"

var ErrNoCR = errors.New("this runc binary has not been compiled with checkpoint/restore support enabled (runc_nocriu)")

func (c *Container) Restore(process *Process, criuOpts *CriuOpts) error {
	return ErrNoCR
}

func (c *Container) Checkpoint(criuOpts *CriuOpts) error {
	return ErrNoCR
}
Repository: opencontainers/runc
File: libcontainer/env.go (Go)

Code before:
		}
		key := kv[:i]
		val := kv[i+1:]
		if saw[key] { // Duplicate.
			continue
		}
		saw[key] = true
		if strings.IndexByte(kv, 0) >= 0 {
			return nil, fmt.Errorf("invalid environment variable %q: contains nul byte (\\x00)", key)
		}
		if key == "PATH" {
			// Needs to be set as it is used for binary lookup.
			if err := os.Setenv("PATH", val); err != nil {
				return nil, err
			}
		}
		// We don't override HOME if it is set to a non-empty value.
		if key == "HOME" && val != "" {
			overrideHome = false
		}
		out = append(out, kv)
	}
	// Restore the original order.
	slices.Reverse(out)

	if overrideHome {
		// Get it from container's /etc/passwd and add.
		home, err := getUserHome(uid)
		if err != nil {
			// For backward compatibility, don't return an error, but merely log it.
			logrus.WithError(err).Debugf("HOME not set in process.env, and getting UID %d homedir failed", uid)
		}

		out = append(out, "HOME="+home)
	}

	return out, nil
}

func getUserHome(uid int) (string, error) {
	const defaultHome = "/" // Default value, return this with any error.

	u, err := user.LookupUid(uid)
	if err != nil {
		// ErrNoPasswdEntries is kinda expected as any UID can be specified.
		if errors.Is(err, user.ErrNoPasswdEntries) {
			err = nil
		}
		return defaultHome, err
	}


Review comment (bug):
There's a bug in here. You are appending empty `HOME=` to `out`, and later you append another `HOME=`.

Should be something like
```go
if key == "HOME" {
  if val == "" {
     // For backward compatibility, ignore empty HOME.
     saw[key] = false
     continue
  }
  overrideHome = false
}
```

Code after:
			return nil, errors.New("invalid environment variable: name cannot be empty")
		}
		key := kv[:i]
		val := kv[i+1:]
		if saw[key] { // Duplicate.
			continue
		}
		saw[key] = true
		if strings.IndexByte(kv, 0) >= 0 {
			return nil, fmt.Errorf("invalid environment variable %q: contains nul byte (\\x00)", key)
		}
		if key == "PATH" {
			// Needs to be set as it is used for binary lookup.
			if err := os.Setenv("PATH", val); err != nil {
				return nil, err
			}
		}
		if key == "HOME" {
			if val != "" {
				homeIsSet = true
			} else {
				// Don't add empty HOME to the environment, we will override it later.
				continue
			}
		}
		out = append(out, kv)
	}
	// Restore the original order.
	slices.Reverse(out)

	// If HOME is not found in env, get it from container's /etc/passwd and add.
	if !homeIsSet {
		home, err := getUserHome(uid)
		if err != nil {
			// For backward compatibility, don't return an error, but merely log it.
			logrus.WithError(err).Debugf("HOME not set in process.env, and getting UID %d homedir failed", uid)
		}

		out = append(out, "HOME="+home)
	}

	return out, nil
}

func getUserHome(uid int) (string, error) {
	const defaultHome = "/" // Default value, return this with any error.

	u, err := user.LookupUid(uid)
	if err != nil {
		// ErrNoPasswdEntries is kinda expected as any UID can be specified.
		if errors.Is(err, user.ErrNoPasswdEntries) {
Repository: opencontainers/runc
File: libcontainer/configs/memorypolicy.go (Go)

Code before:
package configs

import "golang.org/x/sys/unix"

//nolint:revive,staticcheck,nolintlint // ignore ALL_CAPS errors in consts from numaif.h, will match unix.* in the future
const (
	MPOL_DEFAULT = iota
	MPOL_PREFERRED
	MPOL_BIND
	MPOL_INTERLEAVE
	MPOL_LOCAL
	MPOL_PREFERRED_MANY
	MPOL_WEIGHTED_INTERLEAVE

	MPOL_F_STATIC_NODES   = 1 << 15
	MPOL_F_RELATIVE_NODES = 1 << 14
	MPOL_F_NUMA_BALANCING = 1 << 13
)

// LinuxMemoryPolicy contains memory policy configuration.
type LinuxMemoryPolicy struct {
	// Mode specifies memory policy mode without mode flags. See
	// set_mempolicy() documentation for details.
	Mode uint
	// Flags contains mode flags.
	Flags []uint
	// Nodes contains NUMA nodes to which the mode applies.
	Nodes *unix.CPUSet
}


Review comment (refactor):
I think, since mode and flags are designed to be ORed, we can simplify things and use a single uint for `mode | flags`.

Code after:
package configs

import "golang.org/x/sys/unix"

// Memory policy modes and flags as defined in /usr/include/linux/mempolicy.h

//nolint:revive,staticcheck,nolintlint // ignore ALL_CAPS errors in consts from numaif.h, will match unix.* in the future
const (
	MPOL_DEFAULT             = 0
	MPOL_PREFERRED           = 1
	MPOL_BIND                = 2
	MPOL_INTERLEAVE          = 3
	MPOL_LOCAL               = 4
	MPOL_PREFERRED_MANY      = 5
	MPOL_WEIGHTED_INTERLEAVE = 6

	MPOL_F_STATIC_NODES   = 1 << 15
	MPOL_F_RELATIVE_NODES = 1 << 14
	MPOL_F_NUMA_BALANCING = 1 << 13
)

// LinuxMemoryPolicy contains memory policy configuration.
type LinuxMemoryPolicy struct {
	// Mode specifies memory policy mode without mode flags. See
	// set_mempolicy() documentation for details.
	Mode uint `json:"mode,omitempty"`
	// Flags contains mode flags.
	Flags uint `json:"flags,omitempty"`
	// Nodes contains NUMA nodes to which the mode applies.
	Nodes *unix.CPUSet `json:"nodes,omitempty"`
}
Repository: opencontainers/runc
File: update.go (Go)

Code before:
			continue
		}
		// Update weights for existing device.
		if wd.Weight != nil {
			devices[i].Weight = *wd.Weight
		}
		if wd.LeafWeight != nil {
			devices[i].LeafWeight = *wd.LeafWeight
		}
		return devices
	}

	// New device -- append it.
	var weight, leafWeight uint16
	if wd.Weight != nil {
		weight = *wd.Weight
	}
	if wd.LeafWeight != nil {
		leafWeight = *wd.LeafWeight
	}

	return append(devices, cgroups.NewWeightDevice(wd.Major, wd.Minor, weight, leafWeight))
}

func upsertThrottleDevice(devices []*cgroups.ThrottleDevice, td specs.LinuxThrottleDevice) []*cgroups.ThrottleDevice {
	for i, dev := range devices {
		if dev.Major == td.Major && dev.Minor == td.Minor {
			devices[i].Rate = td.Rate
			return devices
		}
	}

	return append(devices, cgroups.NewThrottleDevice(td.Major, td.Minor, td.Rate))
}


Review comment (refactor):
Overall LGTM.

One small suggestion: this loop might be better run in reverse order.

Currently, we don't deduplicate devices when parsing config.json, and we write to the cgroup filesystem one by one. As a result, the final rate value ends up being the last one in the config. For example::
```diff
diff --git a/tests/integration/update.bats b/tests/integration/update.bats
index 9e0c64bd..69496f0f 100644
--- a/tests/integration/update.bats
+++ b/tests/integration/update.bats
@@ -905,6 +905,10 @@ EOF
                echo "==="
                skip "can't get device major number from /proc/partitions (got $major)"
        fi
+       update_config '   .linux.resources.blockIO.throttleReadBpsDevice |= [
+                               { major: '"$major"', minor: 0, rate: 485760 },
+                               { major: '"$major"', minor: 0, rate: 485760 }
+                       ]'
 
        runc run -d --console-socket "$CONSOLE_SOCKET" test_update
        [ "$status" -eq 0 ]
```

Code after:
	// the last one will be used.
	for i, dev := range slices.Backward(devices) {
		if dev.Major != wd.Major || dev.Minor != wd.Minor {
			continue
		}
		// Update weights for existing device.
		if wd.Weight != nil {
			devices[i].Weight = *wd.Weight
		}
		if wd.LeafWeight != nil {
			devices[i].LeafWeight = *wd.LeafWeight
		}
		return devices
	}

	// New device -- append it.
	var weight, leafWeight uint16
	if wd.Weight != nil {
		weight = *wd.Weight
	}
	if wd.LeafWeight != nil {
		leafWeight = *wd.LeafWeight
	}

	return append(devices, cgroups.NewWeightDevice(wd.Major, wd.Minor, weight, leafWeight))
}

func upsertThrottleDevice(devices []*cgroups.ThrottleDevice, td specs.LinuxThrottleDevice) []*cgroups.ThrottleDevice {
	// Iterate backwards because in case of a duplicate
	// the last one will be used.
	for i, dev := range slices.Backward(devices) {
		if dev.Major == td.Major && dev.Minor == td.Minor {
			devices[i].Rate = td.Rate
			return devices
		}
	}

	return append(devices, cgroups.NewThrottleDevice(td.Major, td.Minor, td.Rate))
}
Repository: opencontainers/runc
File: libcontainer/process_linux.go (Go)

Code before:
	if p.cmd.SysProcAttr == nil {
		p.cmd.SysProcAttr = &syscall.SysProcAttr{}
	}
	p.cmd.SysProcAttr.UseCgroupFD = true
	p.cmd.SysProcAttr.CgroupFD = int(fd.Fd())

	return fd, nil
}

func (p *setnsProcess) shouldRetryWithoutCgroupFD(err error) bool {
	switch {
	case !p.cmd.SysProcAttr.UseCgroupFD:
		// CgroupFD was not used.
		return false
	case errors.Is(err, errors.ErrUnsupported):
		// clone3(CLONE_INTO_CGROUP) is not supported (ENOSYS),
		// or cgroup is in the domain invalid state (EOPNOTSUPP).
		return true
	case errors.Is(err, unix.EBUSY):
		// Nesting + domain controllers.
		return true
	case p.rootlessCgroups && errors.Is(err, unix.EACCES):
		// Rootless has no direct access to cgroup.
		return true
	}

	return false
}

func (p *setnsProcess) start() (retErr error) {
	defer p.comm.closeParent()

	fd, err := p.prepareCloneIntoCgroup()
	if err != nil {
		return err
	}

	// Get the "before" value of oom kill count.
	oom, _ := p.manager.OOMKillCount()

	err = p.startWithCPUAffinity()
	if fd != nil {
		fd.Close()
	}
	if err != nil && p.shouldRetryWithoutCgroupFD(err) {
		logrus.Debugf("exec(CLONE_INTO_CGROUP) failed with %v, retrying without it", err)
		p.cmd.SysProcAttr.UseCgroupFD = false
		err = p.startWithCPUAffinity()
	}

	// Close the child-side of the pipes (controlled by child).

Review comment (bug):
I fear we might nor retry in a case we should and then the container will never start for the user and be a big regression. Can we log as debug what was the error before returning false?

So in case that happens, we can ask them to run with debug and check the error they get and we just handle that case?

Code after:

	logrus.Debugf("using CLONE_INTO_CGROUP %q", cgroup)
	if p.cmd.SysProcAttr == nil {
		p.cmd.SysProcAttr = &syscall.SysProcAttr{}
	}
	p.cmd.SysProcAttr.UseCgroupFD = true
	p.cmd.SysProcAttr.CgroupFD = int(fd.Fd())

	return fd, nil
}

func (p *setnsProcess) start() (retErr error) {
	defer p.comm.closeParent()

	fd, err := p.prepareCgroupFD()
	if err != nil {
		return err
	}

	// Get the "before" value of oom kill count.
	oom, _ := p.manager.OOMKillCount()

	err = p.startWithCPUAffinity()
	if fd != nil {
		fd.Close()
	}
	if err != nil && p.cmd.SysProcAttr.UseCgroupFD {
		logrus.Debugf("exec with CLONE_INTO_CGROUP failed: %v; retrying without", err)
		// SysProcAttr.CgroupFD is never used when UseCgroupFD is unset.
		p.cmd.SysProcAttr.UseCgroupFD = false
		err = p.startWithCPUAffinity()
	}

	// Close the child-side of the pipes (controlled by child).
	p.comm.closeChild()
	if err != nil {
		return fmt.Errorf("error starting setns process: %w", err)
	}

	defer func() {
		if retErr != nil {
			if newOom, err := p.manager.OOMKillCount(); err == nil && newOom != oom {
				// Someone in this cgroup was killed, this _might_ be us.
				retErr = fmt.Errorf("%w (possibly OOM-killed)", retErr)
			}
			err := ignoreTerminateErrors(p.terminate())
			if err != nil {
				logrus.WithError(err).Warn("unable to terminate setnsProcess")
			}
		}
	}()
Repository: opencontainers/runc
File: libcontainer/process_linux.go (Go)

Code before:
	}
	return p.addIntoCgroupV1()
}

// prepareCgroupFD sets up p.cmd to use clone3 with CLONE_INTO_CGROUP
// to join cgroup early, in p.cmd.Start. Returns an *os.File which
// must be closed by the caller after p.Cmd.Start return.
func (p *setnsProcess) prepareCgroupFD() (*os.File, error) {
	if !cgroups.IsCgroup2UnifiedMode() {
		return nil, nil
	}

	base := p.manager.Path("")
	if base == "" { // No cgroup to join.
		return nil, nil
	}
	sub := ""
	if p.process.SubCgroupPaths != nil {
		sub = p.process.SubCgroupPaths[""]
	}
	cgroup := path.Join(base, sub)
	if !strings.HasPrefix(cgroup, base) {
		return nil, fmt.Errorf("bad sub cgroup path: %s", sub)
	}

	fd, err := os.OpenFile(cgroup, unix.O_PATH|unix.O_DIRECTORY|unix.O_CLOEXEC, 0)
	if err != nil {
		if p.rootlessCgroups {
			return nil, nil
		}
		return nil, fmt.Errorf("can't open cgroup: %w", err)
	}

	logrus.Debugf("using CLONE_INTO_CGROUP %q", cgroup)
	if p.cmd.SysProcAttr == nil {
		p.cmd.SysProcAttr = &syscall.SysProcAttr{}
	}
	p.cmd.SysProcAttr.UseCgroupFD = true
	p.cmd.SysProcAttr.CgroupFD = int(fd.Fd())

	return fd, nil
}

// shouldRetryWithoutCgroupFD tells if the error returned from p.cmd.Start
// could be caused by using cgroupfd.
func (p *setnsProcess) shouldRetryWithoutCgroupFD(err error) bool {
	if err == nil || !p.cmd.SysProcAttr.UseCgroupFD {
		return false
	}
	logrus.Debugf("exec with CLONE_INTO_CGROUP failed: %v", err)


Review comment (performance):
I would prefer we use `openat2` on the cached cgroupfs fd rather than having this double-check logic -- do we not have access to it anymore with the cgroup package split?

Code after:
	}
	return p.addIntoCgroupV1()
}

// prepareCgroupFD sets up p.cmd to use clone3 with CLONE_INTO_CGROUP
// to join cgroup early, in p.cmd.Start. Returns an *os.File which
// must be closed by the caller after p.Cmd.Start return.
func (p *setnsProcess) prepareCgroupFD() (*os.File, error) {
	if !cgroups.IsCgroup2UnifiedMode() {
		return nil, nil
	}

	base := p.manager.Path("")
	if base == "" { // No cgroup to join.
		return nil, nil
	}
	sub := ""
	if p.process.SubCgroupPaths != nil {
		sub = p.process.SubCgroupPaths[""]
	}
	cgroup := path.Join(base, sub)
	if !strings.HasPrefix(cgroup, base) {
		return nil, fmt.Errorf("bad sub cgroup path: %s", sub)
	}

	fd, err := cgroups.OpenFile(base, sub, unix.O_PATH|unix.O_DIRECTORY|unix.O_CLOEXEC)
	if err != nil {
		if p.rootlessCgroups {
			return nil, nil
		}
		return nil, fmt.Errorf("can't open cgroup: %w", err)
	}

	logrus.Debugf("using CLONE_INTO_CGROUP %q", cgroup)
	if p.cmd.SysProcAttr == nil {
		p.cmd.SysProcAttr = &syscall.SysProcAttr{}
	}
	p.cmd.SysProcAttr.UseCgroupFD = true
	p.cmd.SysProcAttr.CgroupFD = int(fd.Fd())

	return fd, nil
}

func (p *setnsProcess) start() (retErr error) {
	defer p.comm.closeParent()

	fd, err := p.prepareCgroupFD()
	if err != nil {
		return err
	}
Repository: opencontainers/runc
File: utils_linux.go (Go)

Code before:
		process.Stderr = nil
		t := &tty{}
		if !detach {
			if err := t.initHostConsole(); err != nil {
				return nil, err
			}
			parent, child, err := utils.NewSockPair("console")
			if err != nil {
				return nil, err
			}
			process.ConsoleSocket = child
			t.postStart = append(t.postStart, parent, child)
			t.consoleC = make(chan error, 1)
			go func() {
				t.consoleC <- t.recvtty(parent)
			}()
		} else {
			// the caller of runc will handle receiving the console master
			conn, err := net.Dial("unix", sockpath)
			if err != nil {
				return nil, err
			}
			t.postStart = append(t.postStart, conn)
			socket, err := conn.(*net.UnixConn).File()
			if err != nil {
				conn.Close()
				return nil, err
			}
			t.postStart = append(t.postStart, socket)
			process.ConsoleSocket = socket
		}
		return t, nil
	}
	// when runc will detach the caller provides the stdio to runc via runc's 0,1,2
	// and the container's process inherits runc's stdio.
	if detach {
		inheritStdio(process)
		return &tty{}, nil
	}

	config := container.Config()
	rootuid, err := config.HostRootUID()
	if err != nil {
		return nil, err
	}
	rootgid, err := config.HostRootGID()
	if err != nil {
		return nil, err
	}

	return setupProcessPipes(process, rootuid, rootgid)

Review comment (suggestion):
FWIW I prefer the `Err error` pattern for stuff like this, but for this one case it's fine...

Code after:
		process.Stderr = nil
		t := &tty{}
		if !detach {
			if err := t.initHostConsole(); err != nil {
				return nil, err
			}
			parent, child, err := utils.NewSockPair("console")
			if err != nil {
				return nil, err
			}
			process.ConsoleSocket = child
			t.postStart = append(t.postStart, parent, child)
			t.consoleC = make(chan error, 1)
			go func() {
				t.consoleC <- t.recvtty(parent)
			}()
		} else {
			// the caller of runc will handle receiving the console master
			conn, err := net.Dial("unix", sockpath)
			if err != nil {
				return nil, err
			}
			defer func() {
				if Err != nil {
					conn.Close()
				}
			}()
			t.postStart = append(t.postStart, conn)
			socket, err := conn.(*net.UnixConn).File()
			if err != nil {
				return nil, err
			}
			t.postStart = append(t.postStart, socket)
			process.ConsoleSocket = socket
		}
		return t, nil
	}
	// when runc will detach the caller provides the stdio to runc via runc's 0,1,2
	// and the container's process inherits runc's stdio.
	if detach {
		inheritStdio(process)
		return &tty{}, nil
	}

	config := container.Config()
	rootuid, err := config.HostRootUID()
	if err != nil {
		return nil, err
	}
	rootgid, err := config.HostRootGID()
	if err != nil {