Repository: google/gvisor
File: runsc/container/container.go (Go)

Code before:
		s := urpc.NewServer()
		s.Register(&goferToHostRPC{goferPID: pid})
		s.StartHandling(rpcServ)
	}()

	// Count the number of mounts that needs an IO file.
	ioFileCount := 0
	for _, cfg := range c.GoferMountConfs {
		if cfg.ShouldUseLisafs() || cfg.ShouldUseErofs() {
			ioFileCount++
		}
	}

	sandEnds := make([]*os.File, 0, ioFileCount)
	// Track which spec.Mount corresponds to which goferMountConf
	mountIdx := 0
	for i, cfg := range c.GoferMountConfs {
		// Align spec mount with gofer mount conf by skipping non-gofer/non-erofs mounts
		// Skip alignment for root mount (i == 0) because it is not present in spec.Mounts
		if i > 0 {
			for mountIdx < len(c.Spec.Mounts) &&
				!specutils.IsGoferMount(c.Spec.Mounts[mountIdx]) &&
				!specutils.IsErofsMount(c.Spec.Mounts[mountIdx]) {
				mountIdx++
			}
		}

		switch {
		case cfg.ShouldUseLisafs():
			fds, err := unix.Socketpair(unix.AF_UNIX, unix.SOCK_STREAM|unix.SOCK_CLOEXEC, 0)
			if err != nil {
				return nil, nil, nil, nil, err
			}
			sandEnds = append(sandEnds, os.NewFile(uintptr(fds[0]), "sandbox IO FD"))

			goferEnd := os.NewFile(uintptr(fds[1]), "gofer IO FD")
			donations.DonateAndClose("io-fds", goferEnd)

		case cfg.ShouldUseErofs():
			// Get the source for the EROFS image
			var mountSrc string
			if i == 0 {
				// Root mount
				mountSrc = rootfsHint.Mount.Source
			} else {
				// Non-root mount: use the aligned mountIdx
				mountSrc = c.Spec.Mounts[mountIdx].Source
			}
			f, err := os.Open(mountSrc)
			if err != nil {
				return nil, nil, nil, nil, fmt.Errorf("opening EROFS image %q: %v", mountSrc, err)

Review comment (refactor):
I think all these changes here are really complex and hard to follow.

The `c.GoferMountConfs` list is a subset of `c.Spec.Mounts`. It is difficult to iterate configs and track mount index. Everywhere else (as you can see in this PR), we iterate the mounts and track the config index. That approach also requires handling rootfs separately (outside the main loop), but that should be OK:
- rootfs often requires special handling, as you can see below, we need to handle rootfs differently in the `if i==0` branch.
- It is consistent with the rest of the codebase.

So it should be something like:
```
switch {
    case c.GoferMountConfs[0].ShouldUseLisafs():
			refactored code to create socket pair and donate the gofer end and append the sandbox end
    case c.GoferMountConfs[0].ShouldUseErofs():
			f, err := os.Open(rootfsHint.Mount.Source)
			if err != nil {
				return nil, nil, nil, nil, fmt.Errorf("opening rootfs image %q: %v", rootfsHint.Mount.Source, err)
			}
			sandEnds = append(sandEnds, f)
}

cfgIdx := 1
for _, m := c.Spec.Mounts {
  if !specutils.HasMountConfig(m) {
    continue
  }
  switch {
    case c.GoferMountConfs[cfgIdx].ShouldUseLisafs():
			refactored code to create socket pair and donate the gofer end and append the sandbox end
    case c.GoferMountConfs[cfgIdx].ShouldUseErofs():
			f, err := os.Open(m.Source)
			if err != nil {
				return nil, nil, nil, nil, fmt.Errorf("opening EROFS image %q: %v", m.Source, err)
			}
			sandEnds = append(sandEnds, f)
  }
  cfgIdx++
}
```

Code after:
		if pid == 0 {
			rpcServ.Close()
			return
		}
		s := urpc.NewServer()
		s.Register(&goferToHostRPC{goferPID: pid})
		s.StartHandling(rpcServ)
	}()

	// Count the number of mounts that needs an IO file.
	ioFileCount := 0
	for _, cfg := range c.GoferMountConfs {
		if cfg.ShouldUseLisafs() || cfg.ShouldUseErofs() {
			ioFileCount++
		}
	}

	sandEnds := make([]*os.File, 0, ioFileCount)

	// Handle rootfs (index 0)
	switch {
 	case c.GoferMountConfs[0].ShouldUseLisafs():
		if err := createLisafsSocketPair(&sandEnds, &donations); err != nil {
			return nil, nil, nil, nil, err
		}

	case c.GoferMountConfs[0].ShouldUseErofs():
		f, err := os.Open(rootfsHint.Mount.Source)
		if err != nil {
			return nil, nil, nil, nil, fmt.Errorf("opening rootfs image %q: %v", rootfsHint.Mount.Source, err)
		}
		sandEnds = append(sandEnds, f)
	}

	// Handle sub mounts
	cfgIdx := 1
	for _, m := range c.Spec.Mounts {
		if !specutils.HasMountConfig(m) {
			continue
		}

		switch {
		case c.GoferMountConfs[cfgIdx].ShouldUseLisafs():
			if err := createLisafsSocketPair(&sandEnds, &donations); err != nil {
				return nil, nil, nil, nil, err
			}

		case c.GoferMountConfs[cfgIdx].ShouldUseErofs():
			f, err := os.Open(m.Source)
			if err != nil {
				return nil, nil, nil, nil, fmt.Errorf("opening EROFS image %q: %v", m.Source, err)