Logging fix and enhancement (#90)

* Logging fix and enhancement

Corrected issue where loglevel in mbusd.conf wasn't parsed correctly. Added support for logfile in mbusd.conf.  Updated README and mbusd..conf.example to true-up with "-h" output and new logging options.

* Updated README

Sync'd up verbiage in the usage details with the usage summary.

* Additional adjustments to sync up README w/ -h output

README.md

@@ -62,7 +62,7 @@ Usage:
        -d     Instruct mbusd not to fork itself (non-daemonize).
        -L logfile
               Specifies log file name ('-' for logging to STDOUT only, relative path or bare filename
-              will be stored at /var/log, default is /var/log/mbusd.log).
+              will be stored at /var/log, default is /var/log/mbus.log).
        -v level
               Specifies log verbosity level (0 for errors only, 1 for warnings and 2 for informational 
               messages also). If mbusd was compiled in debug mode, valid log levels are up to 9, 
@@ -78,11 +78,11 @@ Usage:
        -S     Enable RS-485 support for given serial port device (Linux only)
        -t     Enable RTS RS-485 data direction control using RTS, active transmit.
        -r     Enable RTS RS-485 data direction control using RTS, active receive.
-       -y file
+       -y sysfsfile
-              Enable RS-485 direction data direction control by writing '1' to file
+              Enable RS-485 direction data direction control by writing '1' to sysfs file
               for transmitter enable and '0' to file for transmitter disable.
-       -Y file
+       -Y sysfsfile
-              Enable RS-485 direction data direction control by writing '0' to file
+              Enable RS-485 direction data direction control by writing '0' to sysfs file
               for transmitter enable and '1' to file for transmitter disable.
        -A address
              Specifies TCP server address to bind (default is 0.0.0.0).

conf/mbusd.conf.example

@@ -4,6 +4,14 @@
 #                                           #
 #############################################
 
+########## Logging settings #############
+
+# Logging verbosity level
+loglevel = 2
+
+# Logfile (fully-qualified path, or filename [stored at /var/log/] or - for STDOUT only)
+logfile = /var/log/mbus.log
+
 ########## Serial port settings #############
 
 # Serial port device name

src/cfg.c

@@ -98,6 +98,20 @@ cfg_ltrim(const char *s)
   return (char *) s;
 }
 
+static int
+cfg_is_empty(char *s)
+{
+  char *p = s + strlen(s);
+  if (strlen(s) == 0)
+    return 1;
+  while (p > s && isspace((unsigned char )(*--p)))
+  { //no-op
+  }
+  if (p == s && isspace((unsigned char )(*p)))
+    return 1;
+  return 0;
+}
+
 int
 cfg_handle_param(char *name, char *value)
 {
@@ -231,7 +245,43 @@ cfg_handle_param(char *name, char *value)
   }
   else if (CFG_NAME_MATCH("loglevel"))
   {
-    cfg.dbglvl = (char)strtol(optarg, NULL, 0);
+    cfg.dbglvl = (char)strtol(value, NULL, 0);
+#  ifdef DEBUG
+    if (!(isdigit(*value)) || cfg.dbglvl < 0 || cfg.dbglvl > 9)
+    { /* report about invalid log level */
+      CFG_ERR("invalid loglevel value: %s (must be 0-9)", value);
+#  else
+    if (!(isdigit(*value)) || cfg.dbglvl < 0 || cfg.dbglvl > 2)
+    { /* report about invalid log level */
+      CFG_ERR("invalid loglevel value: %s (must be 0-2)", value);
+#  endif
+      return(0);
+    }
+  }
+  else if (CFG_NAME_MATCH("logfile"))
+  {
+    if (cfg_is_empty(value))
+    {
+      CFG_ERR("missing logfile value", value);
+      return(0);
+    }
+    else if (*value != '/')
+    {
+      if (*value == '-')
+      {
+        /* logging to file disabled */
+        *cfg.logname = '\0';
+      }
+      else
+      { /* concatenate given log file name with default path */
+        strncpy(cfg.logname, LOGPATH, INTBUFSIZE);
+        strncat(cfg.logname, value, INTBUFSIZE - strlen(cfg.logname));
+      }
+    }
+    else strncpy(cfg.logname, value, INTBUFSIZE);
+
+
+    
 #endif
   }
   else {

src/main.c

@@ -56,6 +56,20 @@ ttydata_t tty;
 /* Connections queue descriptor */
 queue_t queue;
 
+static int
+main_is_empty(char *s)
+{
+  char *p = s + strlen(s);
+  if (strlen(s) == 0)
+    return 1;
+  while (p > s && isspace((unsigned char )(*--p)))
+  { //no-op
+  }
+  if (p == s && isspace((unsigned char )(*p)))
+    return 1;
+  return 0;
+}
+
 #ifndef HAVE_DAEMON
 #include <fcntl.h>
 #include <unistd.h>
@@ -119,7 +133,7 @@ usage(char *exename)
    "             [-R pause] [-W wait] [-T timeout] [-b]\n\n"
    "Options:\n"
    "  -h         : this help\n"
-   "  -d         : don't daemonize\n"
+   "  -d         : don't fork (non-daemonize)\n"
 #ifdef LOG
    "  -L logfile : set log file name (default is %s%s, \n"
    "               '-' for logging to STDOUT only)\n"
@@ -136,14 +150,16 @@ usage(char *exename)
 #ifdef HAVE_TIOCRS485
    "  -S         : enable Linux RS-485 support for given serial port device\n"
 #endif   
-   "  -A address : set TCP server address to bind (default is %s)\n"
-   "  -P port    : set TCP server port number (default is %d)\n"
 #ifdef TRXCTL
    "  -t         : enable RTS RS-485 data direction control using RTS, active transmit\n"
    "  -r         : enable RTS RS-485 data direction control using RTS, active receive\n"
    "  -y         : enable RTS RS-485 data direction control using sysfs file, active transmit\n"
+   "               (writes '1' to sysfs file for transmit enable, '0' for transmit disable)\n"
    "  -Y         : enable RTS RS-485 data direction control using sysfs file, active receive\n"
+   "               (writes '0' to sysfs file for transmit enable, '1' for transmit disable)\n"
 #endif
+   "  -A address : set TCP server address to bind (default is %s)\n"
+   "  -P port    : set TCP server port number (default is %d)\n"
    "  -C maxconn : set maximum number of simultaneous TCP connections\n"
    "               (1-%d, default is %d)\n"
    "  -N retries : set maximum number of request retries\n"
@@ -167,6 +183,7 @@ usage(char *exename)
   exit(0);
 }
 
+
 int
 main(int argc, char *argv[])
 {
@@ -242,21 +259,26 @@ main(int argc, char *argv[])
       case 'v':
         cfg.dbglvl = (char)strtol(optarg, NULL, 0);
 #  ifdef DEBUG
-        if (cfg.dbglvl > 9)
+        if (!(isdigit(*optarg)) || cfg.dbglvl < 0 || cfg.dbglvl > 9)
         { /* report about invalid log level */
           printf("%s: -v: invalid loglevel value"
-                 " (%d, must be 0-9)\n", exename, cfg.dbglvl);
+                 " (%s, must be 0-9)\n", exename, optarg);
 #  else
-        if (cfg.dbglvl < 0 || cfg.dbglvl > 9)
+        if (!(isdigit(*optarg)) || cfg.dbglvl < 0 || cfg.dbglvl > 2)
         { /* report about invalid log level */
           printf("%s: -v: invalid loglevel value"
-                 " (%d, must be 0-2)\n", exename, cfg.dbglvl);
+                 " (%s, must be 0-2)\n", exename, optarg);
 #  endif
           exit(-1);
         }
         break;
       case 'L':
-        if (*optarg != '/')
+        if (main_is_empty(optarg))
+        { /* report about invalid log file */
+          printf("%s: -L: missing logfile value\n", exename, optarg);
+          exit(-1);
+        }
+        else if (*optarg != '/')
         {
           if (*optarg == '-')
           {