Skip to content

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

filename should not be empty for TimeBasedRollingFileAppender #517

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

Merged
merged 1 commit into from
Aug 15, 2021

Conversation

foxhlchen
Copy link
Contributor

TimeBasedRollingFileAppender uses filenamePattern alone with current date time as opened filename and filename field is left empty almost all the time. This brings the problem when it calls append() method from the base class FileAppenderBase. The base class FileAppenderBase assumes that filename field is the path for opened file. Now this contract is broken.

Currently, this will cause log4cplus to print

"log4cplus:ERROR file is not open:"

when out stream is broken. It's confusing and misleading. Users may think that log4cplus tries to open a file with an empty name "".

This does not stop here, as long as it persists, more subtle bugs can be introduced.

This PR fixes this problem by setting filename to the name of the currently opened file in TimeBasedRollingFileAppender::open() making the contract valid again.

TimeBasedRollingFileAppender uses `filenamePattern` alone with
current date time as opened filename and `filename` field is left
empty almost all the time. This brings the problem when it calls
`append()` method from the base class FileAppenderBase.
The base class FileAppenderBase assumes that `filename` field is
the path for opened file. Now this contract is broken.

Currently, this will cause log4cplus to print
"log4cplus: ERROR file is not open:"
when `out` stream is broken. It's confusing and misleading.
Users may think that log4cplus tries to open a file with
the empty name "".

This does not stop here, as long as it persists, more
subtle bugs can be introduced.

This commit fixes this problem by setting filename to the
name of current opened file in
`TimeBasedRollingFileAppender::open()`
making the contract valid again.

Signed-off-by: foxhlchen <foxhlchen@gmail.com>
@wilx wilx self-assigned this Jul 15, 2021
@wilx
Copy link
Contributor

wilx commented Jul 15, 2021

Hm. Shouldn't the TimeBasedRillingFileAppender use the FileAppenderBase::filename field instead of its own scheduledFilename field then? @cfyzium?

@wilx wilx merged commit 06d9b65 into log4cplus:master Aug 15, 2021
@wilx wilx added this to the v2.0.8 milestone Aug 15, 2021
@wilx wilx added the v2.0.8 label Feb 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.