cmcfarlen commented on code in PR #10882:
URL: https://github.com/apache/trafficserver/pull/10882#discussion_r1414213229


##########
src/proxy/logging/LogFile.cc:
##########
@@ -275,31 +275,37 @@ struct RolledFile {
 bool
 LogFile::trim_rolled(size_t rolling_max_count)
 {
-  /* man: "dirname() may modify the contents of path, so it may be
-   * desirable to pass a copy when calling one of these functions." */
+  // man: "dirname() may modify the contents of path, so it may be desirable 
to pass a copy when calling one of these functions."
   char *name = ats_strdup(m_name);
   std::string logfile_dir(::dirname((name)));
   ats_free(name);
 
-  /* Check logging directory access */
+  // Open the directory
+  int dirfd = ::open(logfile_dir.c_str(), O_RDONLY);
+  if (dirfd < 0) {
+    Error("Error opening logging directory %s to collect trim candidates: %s", 
logfile_dir.c_str(), strerror(errno));
+    return false;
+  }
+
+  // Check logging directory access
   int err;
   do {
-    err = access(logfile_dir.c_str(), R_OK | W_OK | X_OK);
+    err = ::faccessat(dirfd, logfile_dir.c_str(), R_OK | W_OK | X_OK, 0);
   } while ((err < 0) && (errno == EINTR));
 
   if (err < 0) {
-    Error("Error accessing logging directory %s: %s.", logfile_dir.c_str(), 
strerror(errno));
+    Error("Error accessing logging directory %s: %s", logfile_dir.c_str(), 
strerror(errno));
     return false;
   }
 
-  /* Open logging directory */
-  DIR *ld = ::opendir(logfile_dir.c_str());
+  // Open the logging directory
+  DIR *ld = ::fdopendir(dirfd);

Review Comment:
   If faccessat or fdopendir fails and we return from this function, I think we 
need to close `dirfd`.  If fdopendir succeeds then I believe the fd will close 
when closedir is called.



-- 
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]

Reply via email to