Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make sure response error when TOC parse failed #10623

Merged
merged 1 commit into from Jul 18, 2023

Conversation

songjiayang
Copy link
Contributor

Signed-off-by: songjiayang songjiayang1@gmail.com

@roidelapluie
Copy link
Member

What are you trying to fix?

@songjiayang
Copy link
Contributor Author

hi @roidelapluie , thanks for your review.

I think d.Error() will always be nil, becuase the d is a new encoding.Decbuf variable. So I want put the error check after d.Be64() , but the bs.length check at the first, so the d.Error() also is nil even put it last.

Copy link
Member

@codesome codesome left a comment

Choose a reason for hiding this comment

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

Nice catch! Just to be explicit about the order and not rely on the compiler ordering, I have suggested a small modification.

tsdb/index/index.go Outdated Show resolved Hide resolved
Signed-off-by: songjiayang <songjiayang1@gmail.com>
@roidelapluie
Copy link
Member

Thanks!

@roidelapluie roidelapluie merged commit 1f5934e into prometheus:main Jul 18, 2023
roidelapluie pushed a commit to roidelapluie/prometheus that referenced this pull request Aug 3, 2023
Signed-off-by: songjiayang <songjiayang1@gmail.com>
candlerb referenced this pull request Sep 30, 2023
Signed-off-by: Julien Pivotto <roidelapluie@o11y.eu>
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.

None yet

3 participants