Skip to content

bins: fix ndpi_set_bin, ndpi_inc_bin and ndpi_get_bin_value#2536

Merged
IvanNardi merged 1 commit into
ntop:devfrom
IvanNardi:bins
Sep 3, 2024
Merged

bins: fix ndpi_set_bin, ndpi_inc_bin and ndpi_get_bin_value#2536
IvanNardi merged 1 commit into
ntop:devfrom
IvanNardi:bins

Conversation

@IvanNardi

Copy link
Copy Markdown
Collaborator

When the required slot is too big, use the latest/bigger available bin, not in the first one.

When the required slot is too big, use the latest/bigger available bin,
not in the first one.
@sonarqubecloud

Copy link
Copy Markdown

@utoni utoni left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Doesn't it make more sense to let the library crash if someone uses it the wrong way instead of modifying already doomed code? 😃

@IvanNardi

IvanNardi commented Sep 3, 2024

Copy link
Copy Markdown
Collaborator Author

Current code doesn't crash; it simply puts "big" values in the first slot.
While using this code to plot some histogram with excel, I found weird that "too big/ out-of-scale" values are shown in the far left of the histogram, instead of in the far right...

@utoni

utoni commented Sep 3, 2024

Copy link
Copy Markdown
Collaborator

Current code doesn't crash; it simply puts "big" values in the first slot.

Sometimes it can be useful to let a library crash, especially if some third party uses the API in the wrong way. But I just saw that there is no documentation in src/include/ndpi_api.h, so I guess it's fine.

@IvanNardi IvanNardi merged commit f2da169 into ntop:dev Sep 3, 2024
@IvanNardi IvanNardi deleted the bins branch September 3, 2024 10:41
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