Skip to content

Commit

Permalink
fix: remove pointer to loop variable when searching the latest event …
Browse files Browse the repository at this point in the history
…to analyze (#328)

Having a pointer to a range variable will always yield the latest value
the loop sees. This leads to subtle bugs. To prevent this from
happening, the range variable was assigned to a temp variable, which is
then referenced as a pointer.

Signed-off-by: Patrick Pichler <git@patrickpichler.dev>
Co-authored-by: Patrick Pichler <git@patrickpichler.dev>
  • Loading branch information
patrickpichler and Patrick Pichler committed Apr 25, 2023
1 parent 3d11e12 commit 2616220
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 3 deletions.
6 changes: 5 additions & 1 deletion cmd/serve/serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,11 @@ var ServeCmd = &cobra.Command{
if aiProvider == nil {
for _, provider := range configAI.Providers {
if backend == provider.Name {
aiProvider = &provider
// he pointer to the range variable is not really an issue here, as there
// is a break right after, but to prevent potential future issues, a temp
// variable is assigned
p := provider
aiProvider = &p
break
}
}
Expand Down
8 changes: 6 additions & 2 deletions pkg/analyzer/events.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,14 @@ func FetchLatestEvent(ctx context.Context, kubernetesClient *kubernetes.Client,
var latestEvent *v1.Event
for _, event := range events.Items {
if latestEvent == nil {
latestEvent = &event
// this is required, as a pointer to a loop variable would always yield the latest value in the range
e := event
latestEvent = &e
}
if event.LastTimestamp.After(latestEvent.LastTimestamp.Time) {
latestEvent = &event
// this is required, as a pointer to a loop variable would always yield the latest value in the range
e := event
latestEvent = &e
}
}
return latestEvent, nil
Expand Down

0 comments on commit 2616220

Please sign in to comment.