From 01831edbd7d6b66b3859fc09e26b6f1b5a1de808 Mon Sep 17 00:00:00 2001
From: Sumera Priyadarsini <sylphrenadin@gmail.com>
Date: Tue, 22 Nov 2022 13:30:03 +0530
Subject: [PATCH] pkg/buildid: Refactor buildid api

- unify path and elf file into an ElfFile struct type
- update references to and tests for BuildID() accordingly

Signed-off-by: Sumera Priyadarsini <sylphrenadin@gmail.com>
---
 pkg/buildid/buildid.go        | 30 +++++++++++-------------------
 pkg/buildid/buildid_test.go   |  4 +++-
 pkg/metadata/compiler.go      |  2 +-
 pkg/objectfile/object_file.go |  2 +-
 pkg/process/maps.go           |  2 +-
 5 files changed, 17 insertions(+), 23 deletions(-)

diff --git a/pkg/buildid/buildid.go b/pkg/buildid/buildid.go
index ca7a4ed5a4..d6ec8d5cf0 100644
--- a/pkg/buildid/buildid.go
+++ b/pkg/buildid/buildid.go
@@ -28,38 +28,36 @@ import (
 	"github.com/parca-dev/parca-agent/pkg/elfreader"
 )
 
-func BuildID(f *elf.File, path string) (string, error) {
-	/*f, err := elf.Open(path)
-	if err != nil {
-		return "", fmt.Errorf("failed to open elf: %w", err)
-	}
-	defer f.Close()
-	*/
+type ElfFile struct {
+	Path string
+	File *elf.File
+}
 
+func BuildID(f *ElfFile) (string, error) {
 	hasGoBuildIDSection := false
-	for _, s := range f.Sections {
+	for _, s := range f.File.Sections {
 		if s.Name == ".note.go.buildid" {
 			hasGoBuildIDSection = true
 		}
 	}
 
 	if hasGoBuildIDSection {
-		if id, err := fastGoBuildID(f); err == nil && len(id) > 0 {
+		if id, err := fastGoBuildID(f.File); err == nil && len(id) > 0 {
 			return hex.EncodeToString(id), nil
 		}
 
-		id, err := gobuildid.ReadFile(path)
+		id, err := gobuildid.ReadFile(f.Path)
 		if err != nil {
-			return elfBuildID(path)
+			return elfBuildID(f.Path)
 		}
 
 		return hex.EncodeToString([]byte(id)), nil
 	}
-	if id, err := fastGNUBuildID(f); err == nil && len(id) > 0 {
+	if id, err := fastGNUBuildID(f.File); err == nil && len(id) > 0 {
 		return hex.EncodeToString(id), nil
 	}
 
-	return elfBuildID(path)
+	return elfBuildID(f.Path)
 }
 
 func fastGoBuildID(f *elf.File) ([]byte, error) {
@@ -105,12 +103,6 @@ func fastGNUBuildID(f *elf.File) ([]byte, error) {
 }
 
 func extractNote(f *elf.File, section string, findBuildID func(notes []elfreader.ElfNote) ([]byte, error)) ([]byte, error) {
-	/*f, err := elf.Open(path)
-	if err != nil {
-		return nil, fmt.Errorf("failed to open elf: %w", err)
-	}
-	defer f.Close()*/
-
 	s := f.Section(section)
 	if s == nil {
 		return nil, fmt.Errorf("failed to find %s section", section)
diff --git a/pkg/buildid/buildid_test.go b/pkg/buildid/buildid_test.go
index 810fdd248f..d9d82dd013 100644
--- a/pkg/buildid/buildid_test.go
+++ b/pkg/buildid/buildid_test.go
@@ -65,7 +65,9 @@ func TestBuildID(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			file, err := elf.Open(tt.args.path)
 			require.NoError(t, err)
-			got, err := BuildID(file, tt.args.path)
+			elfFile := &ElfFile{Path: tt.args.path, File: file}
+
+			got, err := BuildID(elfFile)
 			if tt.wantErr {
 				require.Error(t, err)
 			} else {
diff --git a/pkg/metadata/compiler.go b/pkg/metadata/compiler.go
index dcee6e39d2..2824980dc2 100644
--- a/pkg/metadata/compiler.go
+++ b/pkg/metadata/compiler.go
@@ -68,7 +68,7 @@ func Compiler() Provider {
 			}
 			defer elf.Close()
 
-			buildID, err := buildid.BuildID(elf, path)
+			buildID, err := buildid.BuildID(&buildid.ElfFile{Path: path, File: elf})
 			if err != nil {
 				return nil, fmt.Errorf("buildID failed")
 			}
diff --git a/pkg/objectfile/object_file.go b/pkg/objectfile/object_file.go
index 8224e951e7..27f57504ec 100644
--- a/pkg/objectfile/object_file.go
+++ b/pkg/objectfile/object_file.go
@@ -83,7 +83,7 @@ func open(filePath string, start, limit, offset uint64, relocationSymbol string)
 	defer f.Close()
 
 	buildID := ""
-	if id, err := buildid.BuildID(f, filePath); err == nil {
+	if id, err := buildid.BuildID(&buildid.ElfFile{Path: filePath, File: f}); err == nil {
 		buildID = id
 	}
 
diff --git a/pkg/process/maps.go b/pkg/process/maps.go
index 1b7c1486fc..85d44cc2d6 100644
--- a/pkg/process/maps.go
+++ b/pkg/process/maps.go
@@ -113,7 +113,7 @@ func (c *mappingFileCache) mappingForPID(pid int) ([]*profile.Mapping, error) {
 			}
 			defer fElf.Close()
 
-			m.BuildID, err = buildid.BuildID(fElf, abs)
+			m.BuildID, err = buildid.BuildID(&buildid.ElfFile{Path: abs, File: fElf})
 			if err != nil {
 				if !errors.Is(err, os.ErrNotExist) {
 					level.Debug(c.logger).Log("msg", "failed to read object build ID", "object", abs, "err", err)