Clean up log file handling. Fixes bug #63

We now open the log file from the option parsing (-l file)
OR in mainconfig.c.  That way, the code in log.c can assume that
there is ALWAYS a log file, and it doesn't need to open one.  This
simplifies log.c substantially.

We also moved the syslog "openlog" code from log.c to mainconfig.c
This again makes it simpler.

On HUP, the hup_mainconfig() function takes care of re-opening the
log file.  This is so that the log.c functions don't have to do it.

diff --git a/src/main/event.c b/src/main/event.c
index 918b3e0..c076c43 100644 (file)
--- a/src/main/event.c
+++ b/src/main/event.c
@@ -43,7 +43,6 @@ RCSID("$Id$")
 extern pid_t radius_pid;
 extern int dont_fork;
 extern int check_config;
-extern void force_log_reopen(void);
 extern char *debug_condition;
 
 /*
@@ -3654,13 +3653,14 @@ static void handle_signal_self(int flag)
                time_t when;
                static time_t last_hup = 0;
 
-               radlog(L_INFO, "Received HUP signal.");
-
                when = time(NULL);
                if ((int) (when - last_hup) < 5) {
                        radlog(L_INFO, "Ignoring HUP (less than 5s since last one)");
                        return;
                }
+
+               radlog(L_INFO, "Received HUP signal.");
+
                last_hup = when;
 
                fr_event_loop_exit(el, 0x80);
@@ -3945,12 +3945,6 @@ int radius_event_init(CONF_SECTION *cs, int spawn_flag)
        }
 #endif
 
-       /*
-        *      Just before we spawn the child threads, force the log
-        *      subsystem to re-open the log file for every write.
-        */
-       if (spawn_flag) force_log_reopen();
-
 #ifdef HAVE_PTHREAD_H
 #ifndef __MINGW32__
        NO_SUCH_CHILD_PID = (pthread_t ) (0);

diff --git a/src/main/mainconfig.c b/src/main/mainconfig.c
index 58167da..d067da5 100644 (file)
--- a/src/main/mainconfig.c
+++ b/src/main/mainconfig.c
@@ -606,7 +606,6 @@ static int switch_users(CONF_SECTION *cs)
                 *      things needed inside of the chroot are the
                 *      logging directories.
                 */
-               radlog(L_INFO, "performing chroot to %s\n", chroot_dir);
        }
 
 #ifdef HAVE_GRP_H
@@ -622,23 +621,24 @@ static int switch_users(CONF_SECTION *cs)
        /*
         *      Just before losing root permissions, ensure that the
         *      log files have the correct owner && group.
+        *
+        *      We have to do this because the log file MAY have been
+        *      specified on the command-line.
         */
        if (uid_name || gid_name) {
                if ((mainconfig.radlog_dest == RADLOG_FILES) &&
-                   (mainconfig.log_file != NULL)) {
-                       int fd = open(mainconfig.log_file,
-                                     O_WRONLY | O_APPEND | O_CREAT, 0640);
-                       if (fd < 0) {
-                               fprintf(stderr, "%s: Cannot write to log file %s: %s\n",
-                                       progname, mainconfig.log_file, strerror(errno));
+                   (mainconfig.radlog_fd < 0)) {
+                       mainconfig.radlog_fd = open(mainconfig.log_file,
+                                                   O_WRONLY | O_APPEND | O_CREAT, 0640);
+                       if (mainconfig.radlog_fd < 0) {
+                               fprintf(stderr, "radiusd: Failed to open log file %s: %s\n", mainconfig.log_file, strerror(errno));
                                return 0;
                        }
-                       close(fd);
                
                        if (chown(mainconfig.log_file, server_uid, server_gid) < 0) {
-                         fprintf(stderr, "%s: Cannot change ownership of log file %s: %s\n", 
-                                 progname, mainconfig.log_file, strerror(errno));
-                         return 0;
+                               fprintf(stderr, "%s: Cannot change ownership of log file %s: %s\n", 
+                                       progname, mainconfig.log_file, strerror(errno));
+                               return 0;
                        }
                }
        }               
@@ -833,6 +833,19 @@ int read_mainconfig(int reload)
                                cf_section_free(&cs);
                                return -1;
                        }
+
+                       /*
+                        *      Call openlog only once, when the
+                        *      program starts.
+                        */
+                       openlog(progname, LOG_PID, mainconfig.syslog_facility);
+
+               } else if (mainconfig.radlog_dest == RADLOG_FILES) {
+                       if (!mainconfig.log_file) {
+                               fprintf(stderr, "radiusd: Error: Specified \"files\" as a log destination, but no log filename was given!\n");
+                               cf_section_free(&cs);
+                               return -1;
+                       }
                }
        }
 
@@ -843,6 +856,22 @@ int read_mainconfig(int reload)
        if (!switch_users(cs)) exit(1);
 #endif
 
+       /*
+        *      Open the log file AFTER switching uid / gid.  If we
+        *      did switch uid/gid, then the code in switch_users()
+        *      took care of setting the file permissions correctly.
+        */
+       if ((mainconfig.radlog_dest == RADLOG_FILES) &&
+           (mainconfig.radlog_fd < 0)) {
+               mainconfig.radlog_fd = open(mainconfig.log_file,
+                                           O_WRONLY | O_APPEND | O_CREAT, 0640);
+               if (mainconfig.radlog_fd < 0) {
+                       fprintf(stderr, "radiusd: Failed to open log file %s: %s\n", mainconfig.log_file, strerror(errno));
+                       cf_section_free(&cs);
+                       return -1;
+               }
+       }
+
        /* Initialize the dictionary */
        cp = cf_pair_find(cs, "dictionary");
        if (cp) p = cf_pair_value(cp);
@@ -968,6 +997,8 @@ void hup_mainconfig(void)
        CONF_SECTION *cs;
        char buffer[1024];
 
+       radlog(L_INFO, "HUP - Re-reading configuration files");
+
        /* Read the configuration file */
        snprintf(buffer, sizeof(buffer), "%.200s/%.50s.conf",
                 radius_dir, mainconfig.name);
@@ -994,6 +1025,35 @@ void hup_mainconfig(void)
        cs_cache = cc;
 
        /*
+        *      Re-open the log file.  If we can't, then keep logging
+        *      to the old log file.
+        *
+        *      The "open log file" code is here rather than in log.c,
+        *      because it makes that function MUCH simpler.
+        */
+       if (mainconfig.radlog_dest == RADLOG_FILES) {
+               int fd, old_fd;
+               
+               fd = open(mainconfig.log_file,
+                         O_WRONLY | O_APPEND | O_CREAT, 0640);
+               if (fd >= 0) {
+                       /*
+                        *      Atomic swap. We'd like to keep the old
+                        *      FD around so that callers don't
+                        *      suddenly find the FD closed, and the
+                        *      writes go nowhere.  But that's hard to
+                        *      do.  So... we have the case where a
+                        *      log message *might* be lost on HUP.
+                        */
+                       old_fd = mainconfig.radlog_fd;
+                       mainconfig.radlog_fd = fd;
+                       close(old_fd);
+               }
+       }
+
+       radlog(L_INFO, "HUP - loading modules");
+
+       /*
         *      Prefer the new module configuration.
         */
        module_hup(cf_section_sub_find(cs, "modules"));

diff --git a/src/main/radiusd.c b/src/main/radiusd.c
index 3104d02..75da933 100644 (file)
--- a/src/main/radiusd.c
+++ b/src/main/radiusd.c
@@ -174,6 +174,12 @@ int main(int argc, char *argv[])
                                }
                                mainconfig.log_file = strdup(optarg);
                                mainconfig.radlog_dest = RADLOG_FILES;
+                               mainconfig.radlog_fd = open(mainconfig.log_file,
+                                                           O_WRONLY | O_APPEND | O_CREAT, 0640);
+                               if (mainconfig.radlog_fd < 0) {
+                                       fprintf(stderr, "radiusd: Failed to open log file %s: %s\n", mainconfig.log_file, strerror(errno));
+                                       exit(1);
+                               }
                                break;            
 
                        case 'i':
@@ -397,7 +403,6 @@ int main(int argc, char *argv[])
         */
        while ((rcode = radius_event_process()) == 0x80) {
                radius_stats_init(1);
-               radlog(L_INFO, "Received HUP.");
                hup_mainconfig();
        }