Reload log file on SIGHUP

There's a report about a nightly "crash" when users download, compile
from source, and replace their distro's mindlnad binary.  This is because
the Debian package includes a patch that uses SIGUSR2 to reopen the log
file and sends SIGUSR2 from logrotate instead of just using the
"copytruncate" logrotate option.  Then logrotate sends SIGUSR2 at 6:25AM,
which causes us to abort due to the unhandled signal.

I don't want to sacrifice SIGUSR2 just for log rotation, especially when
we already do some reload operations on SIGHUP.  So to avoid this
Debian/Ubuntu issue, we'll explicitly ignore SIGUSR2, and add log file
reopening to the SIGHUP handler.  Then hopefully a future Debian package
version will remove the SIGUSR2 patch and use SIGHUP instead (or
copytruncate).

Fixes: SF Bug #313 (log rotation kills minidlna service)

log.c

@@ -63,23 +63,41 @@ log_close(void)
 		fclose(log_fp);
 }
 
-int find_matching_name(const char* str, const char* names[]) {
-	if (str == NULL) return -1;
+void
+log_reopen(void)
+{
+	if (log_path[0] && log_fp)
+	{
+		char logfile[1048];
+		snprintf(logfile, sizeof(logfile), "%s/" LOGFILE_NAME, log_path);
+		fclose(log_fp);
+		log_fp = fopen(logfile, "a");
+		DPRINTF(E_INFO, L_GENERAL, "Reopened log file\n");
+	}
+}
 
-	const char* start = strpbrk(str, ",=");
-	int level, c = (start != NULL) ? start - str : strlen(str);
+int find_matching_name(const char* str, const char* names[])
+{
+	const char *start;
+	int level, c;
+
+	if (!str)
+		return -1;
+
+	start = strpbrk(str, ",=");
+	c = start ? start - str : strlen(str);
 	for (level = 0; names[level] != 0; level++) {
-		if (!(strncasecmp(names[level], str, c)))
+		if (!strncasecmp(names[level], str, c))
 			return level;
 	}
 	return -1;
 }
 
 int
-log_init(const char *fname, const char *debug)
+log_init(const char *debug)
 {
 	int i;
-	FILE *fp;
+	FILE *fp = NULL;
 
 	int level = find_matching_name(debug, level_name);
 	int default_log_level = (level == -1) ? _default_log_level : level;
@@ -117,12 +135,15 @@ log_init(const char *fname, const char *debug)
 		}
 	}
 
-	if (!fname)					// use default i.e. stdout
-		return 0;
-
-	if (!(fp = fopen(fname, "a")))
-		return 1;
+	if (log_path[0])
+	{
+		char logfile[1048];
+		snprintf(logfile, sizeof(logfile), "%s/" LOGFILE_NAME, log_path);
+		if (!(fp = fopen(logfile, "a")))
+			return -1;
+	}
 	log_fp = fp;
+
 	return 0;
 }
 

log.h

@@ -44,8 +44,9 @@ enum _log_facility
 };
 
 extern int log_level[L_MAX];
-extern int log_init(const char *fname, const char *debug);
+extern int log_init(const char *debug);
 extern void log_close(void);
+extern void log_reopen(void);
 extern void log_err(int level, enum _log_facility facility, char *fname, int lineno, char *fmt, ...)
 	__attribute__((__format__ (__printf__, 5, 6)));
 

minidlna.c

@@ -215,9 +215,10 @@ static void
 sighup(int sig)
 {
 	signal(sig, sighup);
-	DPRINTF(E_WARN, L_GENERAL, "received signal %d, re-read\n", sig);
+	DPRINTF(E_WARN, L_GENERAL, "received signal %d, reloading\n", sig);
 
 	reload_ifaces(1);
+	log_reopen();
 }
 
 /* record the startup time */
@@ -290,8 +291,7 @@ getfriendlyname(char *buf, int len)
 #ifndef STATIC // Disable for static linking
 	if (!logname)
 	{
-		struct passwd * pwent;
-		pwent = getpwuid(getuid());
+		struct passwd *pwent = getpwuid(geteuid());
 		if (pwent)
 			logname = pwent->pw_name;
 	}
@@ -710,16 +710,15 @@ init(int argc, char **argv)
 			make_dir(path, S_ISVTX|S_IRWXU|S_IRWXG|S_IRWXO);
 			if (access(path, F_OK) != 0)
 				DPRINTF(E_FATAL, L_GENERAL, "Database path not accessible! [%s]\n", path);
-			strncpyt(db_path, path, PATH_MAX);
+			strncpyt(db_path, path, sizeof(db_path));
 			break;
 		case UPNPLOGDIR:
 			path = realpath(ary_options[i].value, buf);
 			if (!path)
-				path = (ary_options[i].value);
-			make_dir(path, S_ISVTX|S_IRWXU|S_IRWXG|S_IRWXO);
-			if (access(path, F_OK) != 0)
-				DPRINTF(E_FATAL, L_GENERAL, "Log path not accessible! [%s]\n", path);
-			strncpyt(log_path, path, PATH_MAX);
+				path = ary_options[i].value;
+			if (snprintf(log_path, sizeof(log_path), "%s", path) > sizeof(log_path))
+				DPRINTF(E_FATAL, L_GENERAL, "Log path too long! [%s]\n", path);
+			make_dir(log_path, S_ISVTX|S_IRWXU|S_IRWXG|S_IRWXO);
 			break;
 		case UPNPLOGLEVEL:
 			log_level = ary_options[i].value;
@@ -816,15 +815,10 @@ init(int argc, char **argv)
 				optionsfile);
 		}
 	}
-	if (log_path[0] == '\0')
-	{
-		if (db_path[0] == '\0')
-			strncpyt(log_path, DEFAULT_LOG_PATH, PATH_MAX);
-		else
-			strncpyt(log_path, db_path, PATH_MAX);
-	}
-	if (db_path[0] == '\0')
-		strncpyt(db_path, DEFAULT_DB_PATH, PATH_MAX);
+	if (!log_path[0])
+		strncpyt(log_path, DEFAULT_LOG_PATH, sizeof(log_path));
+	if (!db_path[0])
+		strncpyt(db_path, DEFAULT_DB_PATH, sizeof(db_path));
 
 	/* command line arguments processing */
 	for (i=1; i<argc; i++)
@@ -1006,38 +1000,31 @@ init(int argc, char **argv)
 	else if (!log_level)
 		log_level = log_str;
 
-	/* Set the default log file path to NULL (stdout) */
-	path = NULL;
+	/* Set the default log to stdout */
 	if (debug_flag)
 	{
 		pid = getpid();
 		strcpy(log_str+65, "maxdebug");
 		log_level = log_str;
+		log_path[0] = '\0';
 	}
 	else if (GETFLAG(SYSTEMD_MASK))
 	{
 		pid = getpid();
+		log_path[0] = '\0';
 	}
 	else
 	{
 		pid = process_daemonize();
-		#ifdef READYNAS
-		unlink("/ramfs/.upnp-av_scan");
-		path = "/var/log/upnp-av.log";
-		#else
 		if (access(db_path, F_OK) != 0)
 			make_dir(db_path, S_ISVTX|S_IRWXU|S_IRWXG|S_IRWXO);
-		snprintf(buf, sizeof(buf), "%s/minidlna.log", log_path);
-		path = buf;
-		#endif
 	}
-	log_init(path, log_level);
+	if (log_init(log_level) < 0)
+		DPRINTF(E_FATAL, L_GENERAL, "Failed to open log file '%s/" LOGFILE_NAME "': %s\n",
+			log_path, strerror(errno));
 
 	if (process_check_if_running(pidfilename) < 0)
-	{
-		DPRINTF(E_ERROR, L_GENERAL, SERVER_NAME " is already running. EXITING.\n");
-		return 1;
-	}	
+		DPRINTF(E_FATAL, L_GENERAL, SERVER_NAME " is already running. EXITING.\n");
 
 	set_startup_time();
 
@@ -1058,6 +1045,8 @@ init(int argc, char **argv)
 		DPRINTF(E_FATAL, L_GENERAL, "Failed to set %s handler. EXITING.\n", "SIGPIPE");
 	if (signal(SIGHUP, &sighup) == SIG_ERR)
 		DPRINTF(E_FATAL, L_GENERAL, "Failed to set %s handler. EXITING.\n", "SIGHUP");
+	if (signal(SIGUSR2, SIG_IGN) == SIG_ERR)
+		DPRINTF(E_FATAL, L_GENERAL, "Failed to set %s handler. EXITING.\n", "SIGUSR2");
 	signal(SIGUSR1, &sigusr1);
 	sa.sa_handler = process_handle_child_termination;
 	if (sigaction(SIGCHLD, &sa, NULL))

upnpglobalvars.c

@@ -81,8 +81,8 @@ const char * minissdpdsocketpath = "/var/run/minissdpd.sock";
 /* UPnP-A/V [DLNA] */
 sqlite3 *db;
 char friendly_name[FRIENDLYNAME_MAX_LEN];
-char db_path[PATH_MAX] = {'\0'};
-char log_path[PATH_MAX] = {'\0'};
+char db_path[1024] = {'\0'};
+char log_path[1024] = {'\0'};
 struct media_dir_s * media_dirs = NULL;
 struct album_art_name_s * album_art_names = NULL;
 volatile short int quitting = 0;

upnpglobalvars.h

@@ -68,6 +68,12 @@
 #define USE_FORK 1
 #define DB_VERSION 11
 
+#ifdef READYNAS
+# define LOGFILE_NAME "upnp-av.log"
+#else
+# define LOGFILE_NAME "minidlna.log"
+#endif
+
 #ifdef ENABLE_NLS
 #define _(string) gettext(string)
 #else
@@ -229,8 +235,8 @@ extern const char *minissdpdsocketpath;
 extern sqlite3 *db;
 #define FRIENDLYNAME_MAX_LEN 64
 extern char friendly_name[];
-extern char db_path[];
-extern char log_path[];
+extern char db_path[1024];
+extern char log_path[1024];
 extern struct media_dir_s *media_dirs;
 extern struct album_art_name_s *album_art_names;
 extern volatile short int quitting;