-
Notifications
You must be signed in to change notification settings - Fork 846
appender: fix max_files integer underflow when set to zero
#3348
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
Conversation
hds
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your change! It's a good idea, I think we should document the special meaning of 0 in the function docs.
| pub fn max_log_files(self, n: usize) -> Self { | ||
| Self { | ||
| max_files: Some(n), | ||
| max_files: Some(n).filter(|&n| n > 0), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should also add a comment stating that setting n to 0 will disable the max files (effectively make it infinite).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your feedback! I've updated the documentation and added comment to the line.
862c784 to
ac5d5af
Compare
ac5d5af to
de426d4
Compare
hds
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Thank you!
max_files integer underflow when set to zeromax_files integer underflow when set to zero
# 0.2.4 (November 30, 2025) ### Added - Prune old files at startup ([#2966]) - Add fallback to file creation date ([#3000]) ### Fixed - Fix `max_files` integer underflow when set to zero ([#3348]) ### Documented - Update tracing-appender docs link to correct docs.rs URL ([#3325]) [#2966]: https://github.com/tokio-rs/tracing/pull/#2966 [#3000]: https://github.com/tokio-rs/tracing/pull/#3000 [#3325]: https://github.com/tokio-rs/tracing/pull/#3325 [#3348]: https://github.com/tokio-rs/tracing/pull/#3348
# 0.2.4 (November 30, 2025) ### Added - Prune old files at startup ([#2966]) - Add fallback to file creation date ([#3000]) - Introduce weekly rotation ([#3218]) ### Fixed - Fix `max_files` integer underflow when set to zero ([#3348]) ### Documented - Update tracing-appender docs link to correct docs.rs URL ([#3325]) [#2966]: https://github.com/tokio-rs/tracing/pull/#2966 [#3000]: https://github.com/tokio-rs/tracing/pull/#3000 [#3218]: https://github.com/tokio-rs/tracing/pull/#3218 [#3325]: https://github.com/tokio-rs/tracing/pull/#3325 [#3348]: https://github.com/tokio-rs/tracing/pull/#3348
# 0.2.4 (November 26, 2025) ### Added - Prune old files at startup ([#2966]) - Add fallback to file creation date ([#3000]) - Introduce weekly rotation ([#3218]) ### Fixed - Fix `max_files` integer underflow when set to zero ([#3348]) ### Documented - Update tracing-appender docs link to correct docs.rs URL ([#3325]) [#2966]: https://github.com/tokio-rs/tracing/pull/#2966 [#3000]: https://github.com/tokio-rs/tracing/pull/#3000 [#3218]: https://github.com/tokio-rs/tracing/pull/#3218 [#3325]: https://github.com/tokio-rs/tracing/pull/#3325 [#3348]: https://github.com/tokio-rs/tracing/pull/#3348 # Please enter the commit message for your changes. Lines starting # with '#' will be kept; you may remove them yourself if you want to. # An empty message aborts the commit. # # Date: Tue Nov 25 12:06:02 2025 +0100 # # On branch hds/tracing-appender-0.2.4 # Your branch is up to date with 'origin/hds/tracing-appender-0.2.4'. # # Changes to be committed: # modified: tracing-appender/CHANGELOG.md # modified: tracing-appender/Cargo.toml # modified: tracing-appender/README.md # # Untracked files: # backport-2025-01.txt # backport-2025-05.txt # backup-Cargo.lock # commit-msg # dingxiangfei-patch2.diff # justfile # tracing_subscriber.d #
# 0.2.4 (November 26, 2025) ### Added - Prune old files at startup ([#2966]) - Add fallback to file creation date ([#3000]) - Introduce weekly rotation ([#3218]) ### Fixed - Fix `max_files` integer underflow when set to zero ([#3348]) ### Documented - Update tracing-appender docs link to correct docs.rs URL ([#3325]) [#2966]: https://github.com/tokio-rs/tracing/pull/#2966 [#3000]: https://github.com/tokio-rs/tracing/pull/#3000 [#3218]: https://github.com/tokio-rs/tracing/pull/#3218 [#3325]: https://github.com/tokio-rs/tracing/pull/#3325 [#3348]: https://github.com/tokio-rs/tracing/pull/#3348
Motivation
When
max_filesset to zero, rolling.rs:695 will cause integer underflow and panicking in debug build. In my opinion the API should prevent this from happening as "zero" is a validusizevalue to pass in.Solution
Currently, in release builds, if
max_filesis set to zero, it behaves just likeu64::MAXonx86_64platforms. Therefore, I decided to simply makezero == None, which should align with the existing behavior but without the panicking. I believe this can be considered as "non-breaking".Feel free to advise if there a more preferred way of fixing this. Any help on how to provide tests would also be appreciated!