Logging fixups -- allow multiple log destinations, always syslog fatals (even before config read), honor logging changes via web interface

src/conf.c

@@ -270,6 +270,9 @@ LL_ITEM *_conf_fetch_item(LL_HANDLE pll, char *section, char *key) {
     LL_ITEM *psection;
     LL_ITEM *pitem;
 
+    if(!pll)
+        return NULL;
+
     if(!(psection = ll_fetch_item(pll,section)))
         return NULL;
 
@@ -427,6 +430,78 @@ int _conf_verify(LL_HANDLE pll) {
     return is_valid;
 }
 
+
+/**
+ * apply an element
+ */
+void _conf_apply_element(char *section, char *key, char *cold, char *cnew) {
+    if((strcmp(section,"general")==0) && (strcmp(key,"logfile")==0)) {
+        /* logfile changed */
+        if((cnew) && strlen(cnew)) {
+            err_setlogfile(cnew);
+            err_setdest(err_getdest() | LOGDEST_LOGFILE);
+        } else {
+            err_setdest(err_getdest() & ~LOGDEST_LOGFILE);
+        }
+    }
+
+    if((strcmp(section,"general")==0) && (strcmp(key,"loglevel")==0)) {
+        /* loglevel changed */
+        err_setlevel(atoi(cnew));
+    }
+}
+
+/**
+ * apply a new config.  This should really be called locked
+ * just prior to applying the config.
+ *
+ * @param pll the "about to be applied" config
+ */
+void _conf_apply(LL_HANDLE pll) {
+    LL_ITEM *psection;
+    LL_ITEM *pitem, *polditem;
+    char *oldvalue;
+    char *newvalue;
+
+    /* get all new and changed items */
+    psection = NULL;
+    while((psection=ll_get_next(pll, psection))) {
+        /* walk through sections */
+        pitem = NULL;
+        while((pitem=ll_get_next(psection->value.as_ll,pitem))) {
+            if(pitem->type == LL_TYPE_STRING) {
+                newvalue = pitem->value.as_string;
+                polditem = _conf_fetch_item(conf_main,
+                                            psection->key,
+                                            pitem->key);
+                oldvalue = NULL;
+                if(polditem)
+                    oldvalue = polditem->value.as_string;
+
+                _conf_apply_element(psection->key, pitem->key,
+                                    oldvalue, newvalue);
+            }
+        }
+    }
+
+    /* now, get deleted items */
+    psection = NULL;
+    while((psection=ll_get_next(conf_main, psection))) {
+        /* walk through sections */
+        pitem = NULL;
+        while((pitem=ll_get_next(psection->value.as_ll,pitem))) {
+            if(pitem->type == LL_TYPE_STRING) {
+                oldvalue = pitem->value.as_string;
+                if(!_conf_fetch_item(pll,psection->key, pitem->key)) {
+                    _conf_apply_element(psection->key, pitem->key,
+                                        oldvalue,NULL);
+                }
+            }
+        }
+    }
+}
+
+
 /**
  * reload the existing config file.
  * 
@@ -736,6 +811,7 @@ int conf_read(char *file) {
     if(_conf_verify(pllnew)) {
         DPRINTF(E_INF,L_CONF,"Loading new config file.\n");
         _conf_lock();
+        _conf_apply(pllnew);
         if(conf_main) {
             ll_destroy(conf_main);
         }
@@ -920,6 +996,8 @@ int conf_set_string(char *section, char *key, char *value, int verify) {
     char **valuearray;
     int index;
     int err;
+    char *oldvalue=NULL;
+    LL_ITEM *polditem;
 
     _conf_lock();
 
@@ -939,6 +1017,15 @@ int conf_set_string(char *section, char *key, char *value, int verify) {
     if(pce)
         key_type = pce->type;
 
+    /* apply the config change, if necessary */
+    if(key_type != CONF_T_MULTICOMMA) {
+        /* let's apply it */
+        polditem = _conf_fetch_item(conf_main,section,key);
+        if(polditem)
+            oldvalue = polditem->value.as_string;
+        _conf_apply_element(section,key,oldvalue,value);
+    }
+
     if(strcmp(value,"") == 0) {
         /* deleting the item */
         pitem = ll_fetch_item(conf_main,section);

src/err.c

@@ -51,7 +51,7 @@
 #endif
 
 static int err_debuglevel=0; /**< current debuglevel, set from command line with -d */
-static int err_logdestination=LOGDEST_STDERR; /**< current log destination */
+static int err_logdest=0; /**< current log destination */
 static char err_filename[PATH_MAX + 1];
 static FILE *err_file=NULL; /**< if logging to file, the handle of that file */
 static pthread_mutex_t err_mutex=PTHREAD_MUTEX_INITIALIZER; /**< for serializing log messages */
@@ -78,22 +78,27 @@ static int _err_unlock(void);
 void err_reopen(void) {
     int err;
 
-    if(err_logdestination != LOGDEST_LOGFILE)
+    if(!(err_logdest & LOGDEST_LOGFILE))
         return;
     
+    _err_lock();
     fclose(err_file);
     err_file = fopen(err_filename,"a");
     if(!err_file) {
         /* what to do when you lose your logging mechanism?  Keep
          * going?
          */
+        _err_unlock();
         err = errno;
-        err_setdest(PACKAGE,LOGDEST_SYSLOG);
+        err_setdest(err_logdest & (~LOGDEST_LOGFILE));
+        err_setdest(err_logdest | LOGDEST_SYSLOG);
+
         DPRINTF(E_LOG,L_MISC,"Could not rotate log file: %s\n",
                 strerror(err));
-    } else {
+        return;
-        DPRINTF(E_LOG,L_MISC,"Rotated logs\n");
     }
+    _err_unlock();
+    DPRINTF(E_LOG,L_MISC,"Rotated logs\n");
 }
 
 /**
@@ -114,13 +119,13 @@ void err_log(int level, unsigned int cat, char *fmt, ...)
     struct tm tm_now;
     time_t tt_now;
 
-    if(level) {
+    if(level > 1) {
         if(level > err_debuglevel)
             return;
 
         if(!(cat & err_debugmask))
             return;
-    } /* we'll *always* process a log level 0 */
+    } /* we'll *always* process a log level 0 or 1 */
 
     va_start(ap, fmt);
     vsnprintf(errbuf, sizeof(errbuf), fmt, ap);
@@ -128,29 +133,26 @@ void err_log(int level, unsigned int cat, char *fmt, ...)
 
     _err_lock(); /* atomic file writes */
 
-    if((!level) && (err_logdestination != LOGDEST_STDERR)) {
+    if((err_logdest & LOGDEST_LOGFILE) && err_file) {
-        fprintf(stderr,"%s",errbuf);
-        fprintf(stderr,"Aborting\n");
-        fflush(stderr); /* shouldn't have to do this? */
-    }
-
-    switch(err_logdestination) {
-    case LOGDEST_LOGFILE:
         tt_now=time(NULL);
         localtime_r(&tt_now,&tm_now);
         strftime(timebuf,sizeof(timebuf),"%Y-%m-%d %T",&tm_now);
         fprintf(err_file,"%s: %s",timebuf,errbuf);
         if(!level) fprintf(err_file,"%s: Aborting\n",timebuf);
         fflush(err_file);
-        break;
+    }
-    case LOGDEST_STDERR:
+    
+    /* always log to stderr on fatal error */
+    if((err_logdest & LOGDEST_STDERR) || (!level)) {
         fprintf(stderr, "%s",errbuf);
         if(!level) fprintf(stderr,"Aborting\n");
-        break;
+    }
-    case LOGDEST_SYSLOG:
+    
+    /* alwyas log fatals to syslog */
+    if(!level) {
+        os_opensyslog(); // (app,LOG_PID,LOG_DAEMON);
         os_syslog(level,errbuf);
-        if(!level) os_syslog(0, "Fatal error... Aborting\n");
+        os_closesyslog();
-        break;
     }
     
     _err_unlock();
@@ -160,15 +162,66 @@ void err_log(int level, unsigned int cat, char *fmt, ...)
     }
 }
  
-/*
+/**
  * simple get/set interface to debuglevel to avoid global
  */
 void err_setlevel(int level) {
+    _err_lock();
     err_debuglevel = level;
+    _err_unlock();
 }
 
+/**
+ * get current debug level
+ */
 int err_getlevel(void) {
-    return err_debuglevel;
+    int level;
+    _err_lock();
+    level = err_debuglevel;
+    _err_unlock();
+
+    return level;
+}
+
+
+/**
+ * get the logfile destination
+ */
+int err_getdest(void) {
+    int dest;
+    
+    _err_lock();
+    dest=err_logdest;
+    _err_unlock();
+
+    return dest;
+}
+
+
+int err_setlogfile(char *file) {
+    _err_lock();
+
+    if(err_logdest & LOGDEST_LOGFILE) {
+        fclose(err_file);
+    }
+
+    memset(err_filename,0,sizeof(err_filename));
+    strncpy(err_filename,file,sizeof(err_filename)-1);
+
+    err_file = fopen(err_filename,"a");
+    if(err_file == NULL) {
+        err_logdest &= ~LOGDEST_LOGFILE;
+
+        os_opensyslog(); // (app,LOG_PID,LOG_DAEMON);
+        os_syslog(1,"Error opening logfile");
+        os_closesyslog();
+        
+        _err_unlock();
+        return FALSE;
+    }
+
+    _err_unlock();
+    return TRUE;
 }
 
 /**
@@ -177,34 +230,21 @@ int err_getlevel(void) {
  * \param app appname (used only for syslog destination)
  * \param destination where to log to \ref log_dests "as defined in err.h"
  */
-void err_setdest(char *cvalue, int destination) {
+void err_setdest(int destination) {
-    if(err_logdestination == destination)
+    fprintf(stderr,"setting dest to %d\n",destination);
+
+    if(err_logdest == destination)
         return;
 
-    switch(err_logdestination) {
+    _err_lock();
-    case LOGDEST_SYSLOG:
+    if((err_logdest & LOGDEST_LOGFILE) &&
-        os_closesyslog();
+       (!(destination & LOGDEST_LOGFILE))) {
-        break;
+        /* used to be logging to file, not any more */
-    case LOGDEST_LOGFILE:
         fclose(err_file);
-        break;
     }
 
-    switch(destination) {
+    err_logdest=destination;
-    case LOGDEST_LOGFILE:
+    _err_unlock();
-        strncpy(err_filename,cvalue,PATH_MAX);
-        err_file=fopen(err_filename,"a");
-        if(err_file==NULL) {
-            fprintf(stderr,"Error opening %s: %s\n",cvalue,strerror(errno));
-            exit(EXIT_FAILURE);
-        }
-        break;
-    case LOGDEST_SYSLOG:
-        os_opensyslog(); // (app,LOG_PID,LOG_DAEMON);
-        break;
-    }
-
-    err_logdestination=destination;
 }
 /**
  * Set the debug mask.  Given a comma separated list, this walks
@@ -224,6 +264,7 @@ extern int err_setdebugmask(char *list) {
     if(!str)
         return 0;
     
+    _err_lock();
     while(1) {
         token=strtok_r(str,",",&last);
         str=NULL;
@@ -250,6 +291,7 @@ extern int err_setdebugmask(char *list) {
 
     DPRINTF(E_INF,L_MISC,"Debug mask is 0x%08x\n",err_debugmask);
     free(tmpstr);
+    _err_unlock();
 
     return 0;
 }

src/err.h

@@ -29,9 +29,9 @@
 #define __ERR_H__
 
 /** @anchor log_dests */
-#define LOGDEST_STDERR       0  /**< Log to stderr */
+#define LOGDEST_STDERR       1  /**< Log to stderr */
-#define LOGDEST_SYSLOG       1  /**< Log to syslog/eventviewer */
+#define LOGDEST_SYSLOG       2  /**< Log to syslog/eventviewer */
-#define LOGDEST_LOGFILE      2  /**< Log to logfile */
+#define LOGDEST_LOGFILE      4  /**< Log to logfile */
 
 /** @anchor log_levels */
 #define E_SPAM         10   /**< Logorrhea! */
@@ -66,10 +66,13 @@
 
 extern void err_log(int level, unsigned int cat, char *fmt, ...);
 extern void err_reopen(void); /** rotate logfile */
-extern void err_setdest(char *cvalue, int destination);
+extern void err_setdest(int destination);
+extern int err_getdest(void);
 extern void err_setlevel(int level);
 extern int err_getlevel(void);
 extern int err_setdebugmask(char *list);
+extern int err_setlogfile(char *file);
+
 /**
  * Print a debugging or log message
  */

src/main.c

@@ -226,7 +226,7 @@ int main(int argc, char *argv[]) {
     int force_non_root=0;
     int skip_initial=1;
     int convert_conf=0;
-    char *logfile,*db_type,*db_parms,*web_root,*runas;
+    char *db_type,*db_parms,*web_root,*runas;
     char **mp3_dir_array;
     char *servername, *iface;
     int index;
@@ -257,6 +257,7 @@ int main(int argc, char *argv[]) {
             break;
         case 'f':
             config.foreground=1;
+            err_setdest(err_getdest() | LOGDEST_STDERR);
             break;
 
         case 'c':
@@ -333,15 +334,6 @@ int main(int argc, char *argv[]) {
 
     DPRINTF(E_LOG,L_MAIN,"Starting with debuglevel %d\n",err_getlevel());
 
-    if(!config.foreground) {
-        if((logfile = conf_alloc_string("general","logfile",NULL)) != NULL) {
-            err_setdest(logfile,LOGDEST_LOGFILE);
-            free(logfile);
-        } else {
-            err_setdest("mt-daapd",LOGDEST_SYSLOG);
-        }
-    }
-
     /* load plugins before we drop privs?  Maybe... let the 
      * plugins do stuff they might need to */
     plugin_init();
@@ -543,8 +535,6 @@ int main(int argc, char *argv[]) {
 
     DPRINTF(E_LOG,L_MAIN,"Done!\n");
 
-    err_setdest(NULL,LOGDEST_STDERR);
-
     os_deinit();
     return EXIT_SUCCESS;
 }