Re-write remove_old_tsversions and greatest_version

Stop deliberately breaking const rules by copying file->name into
dirbuf and truncating it there.  Handle files located in the root
directory properly. Use unlinkat() from POSIX 200809.

diff --git a/lib/isc/log.c b/lib/isc/log.c
index e94f54c43c3241e9c399a2eac61ad0f3bd6639cf..51369af4c0abd2f5332d79b85b2e2ae9c9b88bf7 100644 (file)
--- a/lib/isc/log.c
+++ b/lib/isc/log.c
@@ -21,9 +21,11 @@
 #include <sys/stat.h>
 #include <sys/types.h> /* dev_t FreeBSD 2.1 */
 #include <time.h>
+#include <unistd.h>
 
 #include <isc/atomic.h>
 #include <isc/dir.h>
+#include <isc/errno.h>
 #include <isc/file.h>
 #include <isc/log.h>
 #include <isc/magic.h>
@@ -1011,25 +1013,37 @@ sync_highest_level(isc_log_t *lctx, isc_logconfig_t *lcfg) {
 
 static isc_result_t
 greatest_version(isc_logfile_t *file, int versions, int *greatestp) {
-       char *bname, *digit_end;
-       const char *dirname;
+       char *digit_end;
+       char dirbuf[PATH_MAX + 1];
+       const char *bname;
+       const char *dirname = ".";
        int version, greatest = -1;
-       size_t bnamelen;
        isc_dir_t dir;
        isc_result_t result;
-       char sep = '/';
+       size_t bnamelen;
 
-       /*
-        * It is safe to UNCONST the file.name because it was copied
-        * with isc_mem_strdup().
-        */
-       bname = strrchr(file->name, sep);
+       bname = strrchr(file->name, '/');
        if (bname != NULL) {
-               *bname++ = '\0';
-               dirname = file->name;
+               /*
+                * Copy the complete file name to dirbuf.
+                */
+               size_t len = strlcpy(dirbuf, file->name, sizeof(dirbuf));
+               if (len >= sizeof(dirbuf)) {
+                       result = ISC_R_NOSPACE;
+                       syslog(LOG_ERR, "unable to remove log files: %s",
+                              isc_result_totext(result));
+                       return (result);
+               }
+
+               /*
+                * Truncate after trailing '/' so the code works for
+                * files in the root directory.
+                */
+               bname++;
+               dirbuf[bname - file->name] = '\0';
+               dirname = dirbuf;
        } else {
-               bname = UNCONST(file->name);
-               dirname = ".";
+               bname = file->name;
        }
        bnamelen = strlen(bname);
 
@@ -1040,7 +1054,7 @@ greatest_version(isc_logfile_t *file, int versions, int *greatestp) {
         * Return if the directory open failed.
         */
        if (result != ISC_R_SUCCESS) {
-               goto out;
+               return (result);
        }
 
        while (isc_dir_read(&dir) == ISC_R_SUCCESS) {
@@ -1054,26 +1068,23 @@ greatest_version(isc_logfile_t *file, int versions, int *greatestp) {
                         * Remove any backup files that exceed versions.
                         */
                        if (*digit_end == '\0' && version >= versions) {
-                               char rmfile[PATH_MAX + 1];
-                               int n = snprintf(rmfile, sizeof(rmfile),
-                                                "%s/%s", dirname,
-                                                dir.entry.name);
-                               if (n >= (int)sizeof(rmfile) || n < 0) {
-                                       result = ISC_R_NOSPACE;
-                                       syslog(LOG_ERR,
-                                              "unable to remove log files: %s",
-                                              isc_result_totext(result));
-                                       break;
-                               }
-                               result = isc_file_remove(rmfile);
-                               if (result != ISC_R_SUCCESS &&
-                                   result != ISC_R_NOTFOUND)
-                               {
-                                       syslog(LOG_ERR,
-                                              "unable to remove log file "
-                                              "'%s': %s",
-                                              rmfile,
-                                              isc_result_totext(result));
+                               int n = unlinkat(dirfd(dir.handle),
+                                                dir.entry.name, 0);
+                               if (n < 0) {
+                                       result = isc_errno_toresult(errno);
+                                       if (result != ISC_R_SUCCESS &&
+                                           result != ISC_R_FILENOTFOUND)
+                                       {
+                                               syslog(LOG_ERR,
+                                                      "unable to remove log "
+                                                      "file '%s%s': %s",
+                                                      bname == file->name
+                                                              ? ""
+                                                              : dirname,
+                                                      dir.entry.name,
+                                                      isc_result_totext(
+                                                              result));
+                                       }
                                }
                        } else if (*digit_end == '\0' && version > greatest) {
                                greatest = version;
@@ -1083,16 +1094,7 @@ greatest_version(isc_logfile_t *file, int versions, int *greatestp) {
        isc_dir_close(&dir);
 
        *greatestp = greatest;
-       result = ISC_R_SUCCESS;
-
-out:
-       /*
-        * Replace the file separator if it was taken out.
-        */
-       if (bname != file->name) {
-               *(bname - 1) = sep;
-       }
-       return (result);
+       return (ISC_R_SUCCESS);
 }
 
 static void
@@ -1112,7 +1114,8 @@ insert_sort(int64_t to_keep[], int64_t versions, int64_t version) {
 }
 
 static int64_t
-last_to_keep(int64_t versions, isc_dir_t *dirp, char *bname, size_t bnamelen) {
+last_to_keep(int64_t versions, isc_dir_t *dirp, const char *bname,
+            size_t bnamelen) {
        int64_t to_keep[ISC_LOG_MAX_VERSIONS] = { 0 };
        int64_t version = 0;
 
@@ -1155,25 +1158,37 @@ last_to_keep(int64_t versions, isc_dir_t *dirp, char *bname, size_t bnamelen) {
 
 static isc_result_t
 remove_old_tsversions(isc_logfile_t *file, int versions) {
-       isc_result_t result;
-       char *bname = NULL, *digit_end = NULL;
-       const char *dirname = NULL;
+       char *digit_end;
+       char dirbuf[PATH_MAX + 1];
+       const char *bname;
+       const char *dirname = ".";
        int64_t version, last = INT64_MAX;
-       size_t bnamelen;
        isc_dir_t dir;
-       char sep = '/';
+       isc_result_t result;
+       size_t bnamelen;
 
-       /*
-        * It is safe to UNCONST the file.name because it was copied
-        * with isc_mem_strdup().
-        */
-       bname = strrchr(file->name, sep);
+       bname = strrchr(file->name, '/');
        if (bname != NULL) {
-               *bname++ = '\0';
-               dirname = file->name;
+               /*
+                * Copy the complete file name to dirbuf.
+                */
+               size_t len = strlcpy(dirbuf, file->name, sizeof(dirbuf));
+               if (len >= sizeof(dirbuf)) {
+                       result = ISC_R_NOSPACE;
+                       syslog(LOG_ERR, "unable to remove log files: %s",
+                              isc_result_totext(result));
+                       return (result);
+               }
+
+               /*
+                * Truncate after trailing '/' so the code works for
+                * files in the root directory.
+                */
+               bname++;
+               dirbuf[bname - file->name] = '\0';
+               dirname = dirbuf;
        } else {
-               bname = UNCONST(file->name);
-               dirname = ".";
+               bname = file->name;
        }
        bnamelen = strlen(bname);
 
@@ -1184,61 +1199,45 @@ remove_old_tsversions(isc_logfile_t *file, int versions) {
         * Return if the directory open failed.
         */
        if (result != ISC_R_SUCCESS) {
-               goto out;
+               return (result);
        }
 
        last = last_to_keep(versions, &dir, bname, bnamelen);
 
-       /*
-        * Then we remove all files that we don't want to_keep
-        */
        while (isc_dir_read(&dir) == ISC_R_SUCCESS) {
                if (dir.entry.length > bnamelen &&
                    strncmp(dir.entry.name, bname, bnamelen) == 0 &&
                    dir.entry.name[bnamelen] == '.')
                {
-                       char *ename = &dir.entry.name[bnamelen + 1];
-                       version = strtoull(ename, &digit_end, 10);
+                       version = strtoull(&dir.entry.name[bnamelen + 1],
+                                          &digit_end, 10);
                        /*
                         * Remove any backup files that exceed versions.
                         */
                        if (*digit_end == '\0' && version < last) {
-                               char rmfile[PATH_MAX + 1];
-                               int n = snprintf(rmfile, sizeof(rmfile),
-                                                "%s/%s", dirname,
-                                                dir.entry.name);
-                               if (n >= (int)sizeof(rmfile) || n < 0) {
-                                       result = ISC_R_NOSPACE;
-                                       syslog(LOG_ERR,
-                                              "unable to remove log files: %s",
-                                              isc_result_totext(result));
-                                       break;
-                               }
-                               result = isc_file_remove(rmfile);
-                               if (result != ISC_R_SUCCESS &&
-                                   result != ISC_R_NOTFOUND)
-                               {
-                                       syslog(LOG_ERR,
-                                              "unable to remove log file "
-                                              "'%s': %s",
-                                              rmfile,
-                                              isc_result_totext(result));
+                               int n = unlinkat(dirfd(dir.handle),
+                                                dir.entry.name, 0);
+                               if (n < 0) {
+                                       result = isc_errno_toresult(errno);
+                                       if (result != ISC_R_SUCCESS &&
+                                           result != ISC_R_FILENOTFOUND)
+                                       {
+                                               syslog(LOG_ERR,
+                                                      "unable to remove log "
+                                                      "file '%s%s': %s",
+                                                      bname == file->name
+                                                              ? ""
+                                                              : dirname,
+                                                      dir.entry.name,
+                                                      isc_result_totext(
+                                                              result));
+                                       }
                                }
                        }
                }
        }
-
        isc_dir_close(&dir);
-       result = ISC_R_SUCCESS;
-
-out:
-       /*
-        * Replace the file separator if it was taken out.
-        */
-       if (bname != file->name) {
-               *(bname - 1) = sep;
-       }
-       return (result);
+       return (ISC_R_SUCCESS);
 }
 
 static isc_result_t