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

UI: Fix repeated log entries #11706

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

exeldro
Copy link
Contributor

@exeldro exeldro commented Jan 6, 2025

Description

too_many_repeated_entries did only detect repeated entries when the format pointer stayed the same.
With this change it also detects repeat entries when the format pointer is not the same, but the contents has the same length and the same sum.

Motivation and Context

ffmpeg_log uses a DStr internal for the format of blogva, because of that the pointer is different every call and too_many_repeated_entries would never detect any repeats.

How Has This Been Tested?

On windows 11 by having a plugin blog multiple the same messages with different format pointers

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code has been run through clang-format.
  • I have read the contributing document.
  • My code is not on the master branch.
  • The code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation.

@PatTheMav
Copy link
Member

Wouldn't it be much simpler to bite the bullet and run a simple strncmp on the strings to compare their content? Afaik that's what rsyslog does (it literally just runs string comparison if the string length is the same) and it was added for low-performing systems many years ago (so "performance" of the comparison didn't seem to be an issue).

The given solution has the potential for false positives (as it only looks at length and sum of byte values) and adds more complexity, and seems to make an already convoluted system even more convoluted, whereas a string comparison seems to do exactly what is intended (answering the question "is this message identical to the prior message") and would not require any distinctions between DStr instances or simple const char pointers.

Or is there some special use case that makes using a string comparison not viable here?

@exeldro
Copy link
Contributor Author

exeldro commented Jan 6, 2025

For strncmp you would need to keep a copy of the previous log message which also needs to get a free somewhere.
For false positives to have effect with this fix it would need to have 30 (MAX_REPEATED_LINES) false positives in a row. I think the chance of that happening is negligible.

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