Skip to content

Division by zero panic fix#1643

Closed
artemseleznev wants to merge 1 commit intoClickHouse:mainfrom
Infowatch:main
Closed

Division by zero panic fix#1643
artemseleznev wants to merge 1 commit intoClickHouse:mainfrom
Infowatch:main

Conversation

@artemseleznev
Copy link
Copy Markdown
Contributor

The Svace static analyzer (https://www.ispras.ru/en/technologies/svace/) flagged this code as suspicious: it could lead to a panic (division by zero) when data is empty.

The Svace static analyzer (https://www.ispras.ru/en/technologies/svace/) flagged this code as suspicious: it could lead to a panic (division by zero) when data is empty.
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Sep 3, 2025

CLA assistant check
All committers have signed the CLA.

@mshustov mshustov requested a review from kavirajk September 16, 2025 10:49
}

nulls = make([]uint8, len(data)/col.col.Size)
if col.col.Size == 0 {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can we do something like following?

size := 0
if col.col.Size != 0 {
    size = len(data)/col.col.Size
}
nulls = make([]uint8, size)

Rationale being, this makes it more readable IMHO. Some additional advantages like

  1. Less duplication with make()
  2. Bit simpler control flow
  3. Single place to calculate the value of size.

what do you think?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes, you're right! thanks!

Copy link
Copy Markdown
Contributor

@kavirajk kavirajk left a comment

Choose a reason for hiding this comment

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

thanks @artemseleznev for the PR :)

Left one minor comment. Happy to merge after that.

@kavirajk kavirajk mentioned this pull request Sep 19, 2025
3 tasks
@kavirajk
Copy link
Copy Markdown
Contributor

closing in favor of #1665. Thanks for the contribution @artemseleznev :)

@kavirajk kavirajk closed this Sep 19, 2025
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.

3 participants