Skip to content

Conversation

jschanck
Copy link
Collaborator

No description provided.

@jschanck jschanck requested a review from mozkeeler October 28, 2024 19:21
@@ -504,7 +504,9 @@ func (ld *LogSyncEngine) NewLogWorker(ctx context.Context, ctLogMeta *types.CTLo
}

metricKey := ctLogMeta.MetricKey()
metrics.SetGauge([]string{metricKey, "coverage"}, float32(logObj.MaxEntry-logObj.MinEntry+1)/float32(sth.TreeSize))
if sth != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we already dereference sth on e.g. line 478?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohh, right, not if fetchErr is something...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, sth is only nil if fetchErr is not.

Copy link
Collaborator

@mozkeeler mozkeeler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. It seems like in other places we use fetchErr == nil as the "is-sth-valid" guard, but I'm not sure that's better.

@jschanck jschanck merged commit a6fdc19 into main Oct 28, 2024
3 checks passed
@jschanck jschanck deleted the ct-fetch-nil-ptr branch October 28, 2024 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants