Copilot commented on code in PR #12407:
URL: https://github.com/apache/trafficserver/pull/12407#discussion_r2317241689
##########
src/proxy/logging/LogFile.cc:
##########
@@ -726,13 +726,34 @@ LogFile::check_fd()
if ((stat_check_count % Log::config->file_stat_frequency) == 0) {
//
- // It's time to see if the file really exists. If we can't see
- // the file (via access), then we'll close our descriptor and
- // attempt to re-open it, which will create the file if it's not
- // there.
+ // Check if the file still exists using fstat() on the open file
descriptor.
+ // For regular files that have been unlinked (e.g., by log rotation tools),
+ // st_nlink will be 0. Non-regular files like stderr/stdout will never have
+ // st_nlink == 0, so this approach works safely for all file types.
//
- if (m_name && !LogFile::exists(m_name)) {
- close_file();
+ if (m_name && is_open()) {
+ int fd = get_fd();
+ if (fd >= 0) {
+ struct stat st;
+ if (fstat(fd, &st) == 0) {
+ // If the file has been unlinked, st_nlink will be 0
+ // This only happens for regular files, not special files like
stderr/stdout
+ if (st.st_nlink == 0) {
+ close_file();
+ }
+ } else {
+ // fstat failed, the file descriptor may be invalid
+ // Fall back to path-based existence check
+ if (!LogFile::exists(m_name)) {
+ close_file();
+ }
+ }
+ } else {
+ // No valid fd, fall back to path-based check
+ if (!LogFile::exists(m_name)) {
+ close_file();
+ }
+ }
}
Review Comment:
The nested conditional structure creates excessive complexity with
duplicated fallback logic. Consider extracting the fallback check into a helper
function or restructuring the conditions to reduce nesting and eliminate the
duplicate `LogFile::exists(m_name)` checks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
