Skip to content

Commit

Permalink
*: switch to Go 1.13-safe error handling
Browse files Browse the repository at this point in the history
This includes switching all wrapping of errors to use %w, and using
errors.{As,Is} where appropriate. In addition, enable errorlint in CI,
to make sure we don't regress this.

Without this, users cannot unwrap underlying errors -- which is
particularly problematic for Raw{At,De}tachProgram. runc for instance
needs to detect the existence of BPF_F_REPLACE, but returning a
stringified error makes it harder to deal with.

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
  • Loading branch information
cyphar committed Jun 8, 2021
1 parent f510d07 commit e6026da
Show file tree
Hide file tree
Showing 30 changed files with 105 additions and 100 deletions.
1 change: 1 addition & 0 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ linters:
enable:
- deadcode
- errcheck
- errorlint
- goimports
- gosimple
- govet
Expand Down
8 changes: 4 additions & 4 deletions asm/instruction.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func (ins *Instruction) Unmarshal(r io.Reader, bo binary.ByteOrder) (uint64, err
ins.Constant = int64(bi.Constant)
ins.Dst, ins.Src, err = bi.Registers.Unmarshal(bo)
if err != nil {
return 0, fmt.Errorf("can't unmarshal registers: %s", err)
return 0, fmt.Errorf("can't unmarshal registers: %w", err)
}

if !bi.OpCode.IsDWordLoad() {
Expand Down Expand Up @@ -90,7 +90,7 @@ func (ins Instruction) Marshal(w io.Writer, bo binary.ByteOrder) (uint64, error)

regs, err := newBPFRegisters(ins.Dst, ins.Src, bo)
if err != nil {
return 0, fmt.Errorf("can't marshal registers: %s", err)
return 0, fmt.Errorf("can't marshal registers: %w", err)
}

bpfi := bpfInstruction{
Expand Down Expand Up @@ -501,6 +501,6 @@ func (use *unreferencedSymbolError) Error() string {
// IsUnreferencedSymbol returns true if err was caused by
// an unreferenced symbol.
func IsUnreferencedSymbol(err error) bool {
_, ok := err.(*unreferencedSymbolError)
return ok
var unused *unreferencedSymbolError
return errors.As(err, &unused)
}
4 changes: 2 additions & 2 deletions cmd/bpf2go/compile.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func compile(args compileArgs) error {
}

if err := cmd.Start(); err != nil {
return fmt.Errorf("can't execute %s: %s", args.cc, err)
return fmt.Errorf("can't execute %s: %w", args.cc, err)
}

if depRd != nil {
Expand All @@ -104,7 +104,7 @@ func compile(args compileArgs) error {
}

if err := cmd.Wait(); err != nil {
return fmt.Errorf("%s: %s", args.cc, err)
return fmt.Errorf("%s: %w", args.cc, err)
}

return nil
Expand Down
10 changes: 5 additions & 5 deletions cmd/bpf2go/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ func run(stdout io.Writer, pkg, outputDir string, args []string) (err error) {
if _, err := os.Stat(input); os.IsNotExist(err) {
return fmt.Errorf("file %s doesn't exist", input)
} else if err != nil {
return fmt.Errorf("state %s: %s", input, err)
return fmt.Errorf("state %s: %w", input, err)
}

inputDir, inputFile, err := splitPathAbs(input)
Expand Down Expand Up @@ -193,7 +193,7 @@ func run(stdout io.Writer, pkg, outputDir string, args []string) (err error) {
out: goFile,
})
if err != nil {
return fmt.Errorf("can't write %s: %s", goFileName, err)
return fmt.Errorf("can't write %s: %w", goFileName, err)
}

fmt.Fprintln(stdout, "Wrote", goFileName)
Expand All @@ -204,19 +204,19 @@ func run(stdout io.Writer, pkg, outputDir string, args []string) (err error) {

deps, err := parseDependencies(cwd, &dep)
if err != nil {
return fmt.Errorf("can't read dependency information: %s", err)
return fmt.Errorf("can't read dependency information: %w", err)
}

// There is always at least a dependency for the main file.
deps[0].file = goFileName
depFile, err := adjustDependencies(makeBase, deps)
if err != nil {
return fmt.Errorf("can't adjust dependency information: %s", err)
return fmt.Errorf("can't adjust dependency information: %w", err)
}

depFileName := goFileName + ".d"
if err := ioutil.WriteFile(depFileName, depFile, 0666); err != nil {
return fmt.Errorf("can't write dependency file: %s", err)
return fmt.Errorf("can't write dependency file: %w", err)
}

fmt.Fprintln(stdout, "Wrote", depFileName)
Expand Down
8 changes: 4 additions & 4 deletions cmd/bpf2go/output.go
Original file line number Diff line number Diff line change
Expand Up @@ -217,12 +217,12 @@ type writeArgs struct {
func writeCommon(args writeArgs) error {
obj, err := ioutil.ReadAll(args.obj)
if err != nil {
return fmt.Errorf("read object file contents: %s", err)
return fmt.Errorf("read object file contents: %w", err)
}

spec, err := ebpf.LoadCollectionSpecFromReader(bytes.NewReader(obj))
if err != nil {
return fmt.Errorf("can't load BPF from ELF: %s", err)
return fmt.Errorf("can't load BPF from ELF: %w", err)
}

maps := make(map[string]string)
Expand Down Expand Up @@ -260,7 +260,7 @@ func writeCommon(args writeArgs) error {

var buf bytes.Buffer
if err := commonTemplate.Execute(&buf, &ctx); err != nil {
return fmt.Errorf("can't generate types: %s", err)
return fmt.Errorf("can't generate types: %w", err)
}

return writeFormatted(buf.Bytes(), args.out)
Expand All @@ -278,7 +278,7 @@ func binaryString(buf []byte) string {
func writeFormatted(src []byte, out io.Writer) error {
formatted, err := format.Source(src)
if err != nil {
return fmt.Errorf("can't format source: %s", err)
return fmt.Errorf("can't format source: %w", err)
}

_, err = out.Write(formatted)
Expand Down
8 changes: 4 additions & 4 deletions elf_reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func LoadCollectionSpecFromReader(rd io.ReaderAt) (*CollectionSpec, error) {
// Assign symbols to all the sections we're interested in.
symbols, err := f.Symbols()
if err != nil {
return nil, fmt.Errorf("load symbols: %v", err)
return nil, fmt.Errorf("load symbols: %w", err)
}

for _, symbol := range symbols {
Expand Down Expand Up @@ -198,7 +198,7 @@ func loadLicense(sec *elf.Section) (string, error) {

data, err := sec.Data()
if err != nil {
return "", fmt.Errorf("section %s: %v", sec.Name, err)
return "", fmt.Errorf("section %s: %w", sec.Name, err)
}
return string(bytes.TrimRight(data, "\000")), nil
}
Expand All @@ -210,7 +210,7 @@ func loadVersion(sec *elf.Section, bo binary.ByteOrder) (uint32, error) {

var version uint32
if err := binary.Read(sec.Open(), bo, &version); err != nil {
return 0, fmt.Errorf("section %s: %v", sec.Name, err)
return 0, fmt.Errorf("section %s: %w", sec.Name, err)
}
return version, nil
}
Expand Down Expand Up @@ -324,7 +324,7 @@ func (ec *elfCode) loadInstructions(section *elfSection) (asm.Instructions, uint
for {
var ins asm.Instruction
n, err := ins.Unmarshal(r, ec.ByteOrder)
if err == io.EOF {
if errors.Is(err, io.EOF) {
return insns, offset, nil
}
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion info.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ func scanFdInfoReader(r io.Reader, fields map[string]interface{}) error {
}

if n, err := fmt.Sscanln(parts[1], field); err != nil || n != 1 {
return fmt.Errorf("can't parse field %s: %v", name, err)
return fmt.Errorf("can't parse field %s: %w", name, err)
}

scanned++
Expand Down
12 changes: 6 additions & 6 deletions info_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -231,19 +231,19 @@ func testStats(prog *Program) error {

stats, err := EnableStats(uint32(unix.BPF_STATS_RUN_TIME))
if err != nil {
return fmt.Errorf("failed to enable stats: %v", err)
return fmt.Errorf("failed to enable stats: %w", err)
}
defer stats.Close()

// Program execution with runtime statistics enabled.
// Should increase both runtime and run counter.
if _, _, err := prog.Test(in); err != nil {
return fmt.Errorf("failed to trigger program: %v", err)
return fmt.Errorf("failed to trigger program: %w", err)
}

pi, err := prog.Info()
if err != nil {
return fmt.Errorf("failed to get ProgramInfo: %v", err)
return fmt.Errorf("failed to get ProgramInfo: %w", err)
}

rc, ok := pi.RunCount()
Expand All @@ -267,18 +267,18 @@ func testStats(prog *Program) error {
lt := rt

if err := stats.Close(); err != nil {
return fmt.Errorf("failed to disable statistics: %v", err)
return fmt.Errorf("failed to disable statistics: %w", err)
}

// Second program execution, with runtime statistics gathering disabled.
// Total runtime and run counters are not expected to increase.
if _, _, err := prog.Test(in); err != nil {
return fmt.Errorf("failed to trigger program: %v", err)
return fmt.Errorf("failed to trigger program: %w", err)
}

pi, err = prog.Info()
if err != nil {
return fmt.Errorf("failed to get ProgramInfo: %v", err)
return fmt.Errorf("failed to get ProgramInfo: %w", err)
}

rc, ok = pi.RunCount()
Expand Down
16 changes: 8 additions & 8 deletions internal/btf/btf.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func LoadSpecFromReader(rd io.ReaderAt) (*Spec, error) {

symbols, err := file.Symbols()
if err != nil {
return nil, fmt.Errorf("can't read symbols: %v", err)
return nil, fmt.Errorf("can't read symbols: %w", err)
}

variableOffsets := make(map[variable]uint32)
Expand Down Expand Up @@ -152,7 +152,7 @@ func loadSpecFromVmlinux(rd io.ReaderAt) (*Spec, error) {

btfSection, _, _, err := findBtfSections(file)
if err != nil {
return nil, fmt.Errorf(".BTF ELF section: %s", err)
return nil, fmt.Errorf(".BTF ELF section: %w", err)
}
if btfSection == nil {
return nil, fmt.Errorf("unable to find .BTF ELF section")
Expand Down Expand Up @@ -250,14 +250,14 @@ func loadKernelSpec() (*Spec, error) {
func parseBTF(btf io.ReadSeeker, bo binary.ByteOrder) ([]rawType, stringTable, error) {
rawBTF, err := ioutil.ReadAll(btf)
if err != nil {
return nil, nil, fmt.Errorf("can't read BTF: %v", err)
return nil, nil, fmt.Errorf("can't read BTF: %w", err)
}

rd := bytes.NewReader(rawBTF)

var header btfHeader
if err := binary.Read(rd, bo, &header); err != nil {
return nil, nil, fmt.Errorf("can't read header: %v", err)
return nil, nil, fmt.Errorf("can't read header: %w", err)
}

if header.Magic != btfMagic {
Expand All @@ -278,11 +278,11 @@ func parseBTF(btf io.ReadSeeker, bo binary.ByteOrder) ([]rawType, stringTable, e
}

if _, err := io.CopyN(internal.DiscardZeroes{}, rd, remainder); err != nil {
return nil, nil, fmt.Errorf("header padding: %v", err)
return nil, nil, fmt.Errorf("header padding: %w", err)
}

if _, err := rd.Seek(int64(header.HdrLen+header.StringOff), io.SeekStart); err != nil {
return nil, nil, fmt.Errorf("can't seek to start of string section: %v", err)
return nil, nil, fmt.Errorf("can't seek to start of string section: %w", err)
}

rawStrings, err := readStringTable(io.LimitReader(rd, int64(header.StringLen)))
Expand All @@ -291,7 +291,7 @@ func parseBTF(btf io.ReadSeeker, bo binary.ByteOrder) ([]rawType, stringTable, e
}

if _, err := rd.Seek(int64(header.HdrLen+header.TypeOff), io.SeekStart); err != nil {
return nil, nil, fmt.Errorf("can't seek to start of type section: %v", err)
return nil, nil, fmt.Errorf("can't seek to start of type section: %w", err)
}

rawTypes, err := readTypes(io.LimitReader(rd, int64(header.TypeLen)), bo)
Expand Down Expand Up @@ -405,7 +405,7 @@ func (s *Spec) marshal(opts marshalOpts) ([]byte, error) {
raw := buf.Bytes()
err := binary.Write(sliceWriter(raw[:headerLen]), opts.ByteOrder, header)
if err != nil {
return nil, fmt.Errorf("can't write header: %v", err)
return nil, fmt.Errorf("can't write header: %w", err)
}

return raw, nil
Expand Down
7 changes: 4 additions & 3 deletions internal/btf/btf_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package btf

import (
"encoding/binary"
"errors"
"fmt"
"io"
)
Expand Down Expand Up @@ -218,10 +219,10 @@ func readTypes(r io.Reader, bo binary.ByteOrder) ([]rawType, error) {
)

for id := TypeID(1); ; id++ {
if err := binary.Read(r, bo, &header); err == io.EOF {
if err := binary.Read(r, bo, &header); errors.Is(err, io.EOF) {
return types, nil
} else if err != nil {
return nil, fmt.Errorf("can't read type info for id %v: %v", id, err)
return nil, fmt.Errorf("can't read type info for id %v: %w", id, err)
}

var data interface{}
Expand Down Expand Up @@ -259,7 +260,7 @@ func readTypes(r io.Reader, bo binary.ByteOrder) ([]rawType, error) {
}

if err := binary.Read(r, bo, data); err != nil {
return nil, fmt.Errorf("type id %d: kind %v: can't read %T: %v", id, header.Kind(), data, err)
return nil, fmt.Errorf("type id %d: kind %v: can't read %T: %w", id, header.Kind(), data, err)
}

types = append(types, rawType{header, data})
Expand Down
4 changes: 2 additions & 2 deletions internal/btf/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,7 @@ func parseCoreAccessor(accessor string) (coreAccessor, error) {
// 31 bits to avoid overflowing int on 32 bit platforms.
index, err := strconv.ParseUint(part, 10, 31)
if err != nil {
return nil, fmt.Errorf("accessor index %q: %s", part, err)
return nil, fmt.Errorf("accessor index %q: %w", part, err)
}

result = append(result, int(index))
Expand Down Expand Up @@ -559,7 +559,7 @@ func coreFindField(local Type, localAcc coreAccessor, target Type) (_, _ coreFie
if localMember.Name == "" {
_, ok := localMember.Type.(composite)
if !ok {
return coreField{}, coreField{}, fmt.Errorf("unnamed field with type %s: %s", localMember.Type, ErrNotSupported)
return coreField{}, coreField{}, fmt.Errorf("unnamed field with type %s: %w", localMember.Type, ErrNotSupported)
}

// This is an anonymous struct or union, ignore it.
Expand Down
Loading

0 comments on commit e6026da

Please sign in to comment.