From: Alan T. DeKok Date: Fri, 26 Feb 2010 10:11:02 +0000 (+0100) Subject: Clean up log file handling. Fixes bug #63 X-Git-Tag: release_2_1_9~78 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6563dac620fdee7bfe634c94e3adc422a078d6ee;p=thirdparty%2Ffreeradius-server.git 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 a20f62902a1..bb650e0c792 100644 --- 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; /* @@ -3230,13 +3229,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); @@ -3484,12 +3484,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 58167da4adc..d067da56592 100644 --- 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); @@ -993,6 +1024,35 @@ void hup_mainconfig(void) cc->next = cs_cache; 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. */ diff --git a/src/main/radiusd.c b/src/main/radiusd.c index 3104d02b212..75da93349a9 100644 --- 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(); }