internal/symbols: remove logging from Patched
Bubble up the errors instead of passing a logging function.
Change-Id: Id55c3588170ae822b09ede3ed0e0224089be147b
Reviewed-on: https://go-review.googlesource.com/c/vulndb/+/560777
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Maceo Thompson <maceothompson@google.com>
Reviewed-by: Zvonimir Pavlinovic <zpavlinovic@google.com>
diff --git a/cmd/vulnreport/symbols.go b/cmd/vulnreport/symbols.go
index 852d397..11ba99c 100644
--- a/cmd/vulnreport/symbols.go
+++ b/cmd/vulnreport/symbols.go
@@ -56,7 +56,7 @@
for i, fixLink := range defaultFixes {
fixHash := filepath.Base(fixLink)
fixRepo := strings.TrimSuffix(fixLink, "/commit/"+fixHash)
- pkgsToSymbols, err := symbols.Patched(mod.Module, fixRepo, fixHash, log.Errf)
+ pkgsToSymbols, err := symbols.Patched(mod.Module, fixRepo, fixHash)
if err != nil {
log.Err(err)
continue
diff --git a/internal/symbols/patched_functions.go b/internal/symbols/patched_functions.go
index 7704db4..21c6264 100644
--- a/internal/symbols/patched_functions.go
+++ b/internal/symbols/patched_functions.go
@@ -35,7 +35,7 @@
// patched in the package. Test packages and symbols are omitted.
//
// If the commit has more than one parent, an error is returned.
-func Patched(module, repoURL, commitHash string, errf logf) (_ map[string][]string, err error) {
+func Patched(module, repoURL, commitHash string) (_ map[string][]string, err error) {
defer derrors.Wrap(&err, "Patched(%s, %s, %s)", module, repoURL, commitHash)
repoRoot, err := os.MkdirTemp("", commitHash)
@@ -90,7 +90,10 @@
return nil, err
}
- patched := patchedSymbols(oldSymbols, newSymbols, errf)
+ patched, err := patchedSymbols(oldSymbols, newSymbols)
+ if err != nil {
+ return nil, err
+ }
pkgSyms := make(map[string][]string)
for _, sym := range patched {
pkgSyms[sym.pkg] = append(pkgSyms[sym.pkg], sym.symbol)
@@ -101,7 +104,7 @@
// patchedSymbols returns symbol indices in oldSymbols that either 1) cannot
// be identified in newSymbols or 2) the corresponding functions have their
// source code changed.
-func patchedSymbols(oldSymbols, newSymbols map[symKey]*ast.FuncDecl, errf logf) []symKey {
+func patchedSymbols(oldSymbols, newSymbols map[symKey]*ast.FuncDecl) ([]symKey, error) {
var syms []symKey
for key, of := range oldSymbols {
nf, ok := newSymbols[key]
@@ -112,23 +115,30 @@
continue
}
- if source(of, errf) != source(nf, errf) {
+ osrc, err := source(of)
+ if err != nil {
+ return nil, err
+ }
+ nsrc, err := source(nf)
+ if err != nil {
+ return nil, err
+ }
+
+ if osrc != nsrc {
syms = append(syms, key)
}
}
- return syms
+ return syms, nil
}
// source returns f's source code as text.
-func source(f *ast.FuncDecl, errf logf) string {
+func source(f *ast.FuncDecl) (string, error) {
var b bytes.Buffer
fs := token.NewFileSet()
if err := printer.Fprint(&b, fs, f); err != nil {
- // should not happen, so just printing a warning
- errf("getting source of %s failed with %v", astSymbolName(f), err)
- return ""
+ return "", fmt.Errorf("getting source of %s failed: %w", astSymbolName(f), err)
}
- return strings.TrimSpace(b.String())
+ return strings.TrimSpace(b.String()), nil
}
// moduleSymbols indexes all symbols of a module located
diff --git a/internal/symbols/patched_functions_test.go b/internal/symbols/patched_functions_test.go
index 1dfd3d2..a85d7ea 100644
--- a/internal/symbols/patched_functions_test.go
+++ b/internal/symbols/patched_functions_test.go
@@ -45,9 +45,11 @@
if err != nil {
t.Error(err)
}
- // logs are not important for this test
- discardLog := func(string, ...any) {}
- got := toMap(patchedSymbols(oldSyms, newSyms, discardLog))
+ patched, err := patchedSymbols(oldSyms, newSyms)
+ if err != nil {
+ t.Fatal(err)
+ }
+ got := toMap(patched)
if diff := cmp.Diff(got, tc.want); diff != "" {
t.Errorf("(-got, want+):\n%s", diff)
}
