Unifying logging in chanserv

This commit adds:
* replace multiple logging macros with LOG and LOG_DEVEL
* logging configuration for chanserv
* logging configuration for console output
* logging configuration for per file or method log level filtering for
debug builds
* file, line, and method name in log message for debug builds

common/log.h

@@ -22,19 +22,21 @@
 #include <pthread.h>
 
 #include "arch.h"
+#include "list.h"
 
 /* logging buffer size */
 #define LOG_BUFFER_SIZE      1024
+#define LOGGER_NAME_SIZE     50
 
 /* logging levels */
 enum logLevels
 {
     LOG_LEVEL_ALWAYS = 0,
-    LOG_LEVEL_ERROR,
-    LOG_LEVEL_WARNING,
-    LOG_LEVEL_INFO,
-    LOG_LEVEL_DEBUG,
-    LOG_LEVEL_TRACE
+    LOG_LEVEL_ERROR,     /* for describing non-recoverable error states in a request or method */
+    LOG_LEVEL_WARNING,   /* for describing recoverable error states in a request or method */
+    LOG_LEVEL_INFO,      /* for low verbosity and high level descriptions of normal operations */
+    LOG_LEVEL_DEBUG,     /* for medium verbosity and low level descriptions of normal operations */
+    LOG_LEVEL_TRACE      /* for high verbosity and low level descriptions of normal operations (eg. method or wire tracing) */
 };
 
 /* startup return values */
@@ -49,29 +51,92 @@ enum logReturns
     LOG_GENERAL_ERROR
 };
 
-#define SESMAN_CFG_LOGGING           "Logging"
-#define SESMAN_CFG_LOG_FILE          "LogFile"
-#define SESMAN_CFG_LOG_LEVEL         "LogLevel"
-#define SESMAN_CFG_LOG_ENABLE_SYSLOG "EnableSyslog"
-#define SESMAN_CFG_LOG_SYSLOG_LEVEL  "SyslogLevel"
+#define SESMAN_CFG_LOGGING            "Logging"
+#define SESMAN_CFG_LOGGING_LOGGER     "LoggingPerLogger"
+#define SESMAN_CFG_LOG_FILE           "LogFile"
+#define SESMAN_CFG_LOG_LEVEL          "LogLevel"
+#define SESMAN_CFG_LOG_ENABLE_CONSOLE "EnableConsole"
+#define SESMAN_CFG_LOG_CONSOLE_LEVEL  "ConsoleLevel"
+#define SESMAN_CFG_LOG_ENABLE_SYSLOG  "EnableSyslog"
+#define SESMAN_CFG_LOG_SYSLOG_LEVEL   "SyslogLevel"
+#define SESMAN_CFG_LOG_ENABLE_PID     "EnableProcessId"
 
 /* enable threading */
 /*#define LOG_ENABLE_THREAD*/
 
 #ifdef XRDP_DEBUG
-#define LOG_DBG(args...) log_message(LOG_LEVEL_DEBUG, args);
+
+/**
+ * @brief Logging macro for messages that are for an XRDP developper to 
+ * understand and debug XRDP code.
+ * 
+ * Note: all log levels are relavant to help a developper understand XRDP at 
+ *      different levels of granularity.
+ * 
+ * Note: the logging function calls are removed when XRDP_DEBUG is NOT defined.
+ * 
+ * Note: when the build is configured with --enable-xrdpdebug, then 
+ *      the log level can be configured per the source file name or method name 
+ *      (with the suffix "()") in the [LoggingPerLogger]
+ *      section of the configuration file.
+ * 
+ *      For example:
+ *      ```     
+ *      [LoggingPerLogger]
+ *      xrdp.c=DEBUG
+ *      main()=WARNING
+ *      ```
+ * 
+ * @param lvl, the log level
+ * @param msg, the log text as a printf format c-string
+ * @param ... the arguments for the printf format c-string
+ */
+#define LOG_DEVEL(log_level, args...) \
+        log_message_with_location(__func__, __FILE__, __LINE__, log_level, args);
+
+/**
+ * @brief Logging macro for messages that are for a systeam administrator to
+ * configure and run XRDP on their machine.
+ * 
+ * Note: the logging function calls contain additional code location info when 
+ *      XRDP_DEBUG is defined.
+ * 
+ * @param lvl, the log level
+ * @param msg, the log text as a printf format c-string
+ * @param ... the arguments for the printf format c-string
+ */
+#define LOG(log_level, args...) \
+        log_message_with_location(__func__, __FILE__, __LINE__, log_level, args);
 #else
-#define LOG_DBG(args...)
+#define LOG_DEVEL(log_level, args...)
+#define LOG(log_level, args...) log_message(log_level, args);
 #endif
 
+enum log_logger_type
+{
+    LOG_TYPE_FILE = 0,
+    LOG_TYPE_FUNCTION,
+};
+
+struct log_logger_level
+{
+    enum logLevels log_level;
+    enum log_logger_type logger_type;
+    char logger_name[LOGGER_NAME_SIZE + 1];
+};
+
 struct log_config
 {
     const char *program_name;
     char *log_file;
     int fd;
     enum logLevels log_level;
+    int enable_console;
+    enum logLevels console_level;
     int enable_syslog;
     enum logLevels syslog_level;
+    struct list *per_logger_level;
+    int enable_pid;
     pthread_mutex_t log_lock;
     pthread_mutexattr_t log_lock_attr;
 };
@@ -121,25 +186,28 @@ internal_log_text2level(const char *buf);
  * also init its content.
  * @return  LOG_STARTUP_OK or LOG_ERROR_MALLOC
  */
-enum logReturns
+struct log_config*
 internalInitAndAllocStruct(void);
 
 /**
- * Read configuration from a file and store the values in lists.
- * @param file
- * @param lc
- * @param param_n
- * @param param_v
- * @param applicationName, the application name used in the log events.
+ * Print the contents of the logging config to stdout.
+ */
+void
+internal_log_config_dump(struct log_config *config);
+
+/**
+ * the log function that all files use to log an event.
+ * @param lvl, the loglevel
+ * @param msg, the logtext.
+ * @param ...
  * @return
  */
 enum logReturns
-internal_config_read_logging(int file, struct log_config *lc,
-                             struct list *param_n,
-                             struct list *param_v,
-                             const char *applicationName);
+internal_log_message(const enum logLevels lvl, bool_t force_log, const char *msg, va_list args);
+
 /*End of internal functions*/
 #endif
+
 /**
  * This function initialize the log facilities according to the configuration
  * file, that is described by the in parameter.
@@ -152,11 +220,34 @@ log_start(const char *iniFile, const char *applicationName);
 
 /**
  * An alternative log_start where the caller gives the params directly.
- * @param iniParams
+ * @param config
  * @return
+ * 
+ * @post to avoid memory leaks, the config argument must be free'ed using 
+ * `log_config_free()`
  */
 enum logReturns
-log_start_from_param(const struct log_config *iniParams);
+log_start_from_param(const struct log_config *src_log_config);
+
+/**
+ * Read configuration from a file and store the values in the returned 
+ * log_config.
+ * @param file
+ * @param applicationName, the application name used in the log events.
+ * @param section_prefix, prefix for the logging sections to parse
+ * @return
+ */
+struct log_config*
+log_config_init_from_config(const char *iniFilename, 
+                            const char *applicationName, 
+                            const char *section_prefix);
+
+/**
+ * Free the memory for the log_config struct.
+ */
+enum logReturns
+log_config_free(struct log_config* config);
+
 /**
  * Function that terminates all logging
  * @return
@@ -166,6 +257,9 @@ log_end(void);
 
 /**
  * the log function that all files use to log an event.
+ * 
+ * Please prefer to use the LOG and LOG_DEVEL macros instead of this function directly.
+ * 
  * @param lvl, the loglevel
  * @param msg, the logtext.
  * @param ...
@@ -174,6 +268,28 @@ log_end(void);
 enum logReturns
 log_message(const enum logLevels lvl, const char *msg, ...) printflike(2, 3);
 
+/**
+ * the log function that all files use to log an event, 
+ * with the function name and file line.
+ *
+ * Please prefer to use the LOG and LOG_DEVEL macros instead of this function directly.
+ * 
+ * @param function_name, the function name (typicaly the __func__ macro)
+ * @param file_name, the file name (typicaly the __FILE__ macro)
+ * @param line_number, the line number in the file (typicaly the __LINE__ macro)
+ * @param lvl, the loglevel
+ * @param msg, the logtext.
+ * @param ...
+ * @return
+ */
+enum logReturns
+log_message_with_location(const char *function_name, 
+                          const char *file_name, 
+                          const int line_number, 
+                          const enum logLevels lvl, 
+                          const char *msg, 
+                          ...) printflike(5, 6);
+
 /**
  * This function returns the configured file name for the logfile
  * @param replybuf the buffer where the reply is stored

docs/man/sesman.ini.5.in

@@ -32,6 +32,10 @@ X11 server settings for supported servers
 \fB[Chansrv]\fR
 Settings for xrdp-chansrv(8)
 
+.TP
+\fB[Chansrv_Logging]\fR
+Logging settings for xrdp-chansrv(8)
+
 .TP
 \fB[SessionVariables]\fR
 Environment variables for the session
@@ -81,12 +85,15 @@ relative path to \fI@xrdpconfdir@\fR. If not specified, defaults to
 \fI@xrdpconfdir@/reconnectwm.sh\fR.
 
 .SH "LOGGING"
-Following parameters can be used in the \fB[Logging]\fR section.
+Following parameters can be used in the \fB[Logging]\fR and \fB[ChansrvLogging]\fR 
+sections.
 
 .TP
 \fBLogFile\fR=\fIfilename\fR
 Log file path. It can be either absolute or relative. If not specified,
-defaults to \fI./sesman.log\fR
+defaults to \fI./sesman.log\fR It is ignored in the [ChansrvLogging] section
+since the channel server creates one log file per display and instead uses the
+following log file naming convention \fIxrdp-chansrv.${DISPLAY}.log\fR
 
 .TP
 \fBLogLevel\fR=\fIlevel\fR
@@ -112,8 +119,22 @@ syslog.
 .TP
 \fBSyslogLevel\fR=\fIlevel\fR
 Logging level for syslog. It can have the same values as \fBLogLevel\fR.
-If \fBSyslogLevel\fR and \fBLogLevel\fR differ, the least verbose setting
-takes effect for syslog.
+Defaults to \fBDEBUG\fR.
+
+.TP
+\fBEnableConsole\fR=\fI[true|false]\fR
+If set to \fB1\fR, \fBtrue\fR or \fByes\fR, this option enables logging to
+the console (ie. stdout).
+
+.TP
+\fBConsoleLevel\fR=\fIlevel\fR
+Logging level for the console. It can have the same values as \fBLogLevel\fR.
+Defaults to \fBDEBUG\fR.
+
+.TP
+\fBEnableProcessId\fR=\fI[true|false]\fR
+If set to \fB1\fR, \fBtrue\fR or \fByes\fR, this option enables logging the
+process id in all log messages. Defaults to \fBfalse\fR.
 
 .SH "SESSIONS"
 Following parameters can be used in the \fB[Sessions]\fR section.

docs/man/xrdp-chansrv.8.in

@@ -31,30 +31,6 @@ Dynamic Virtual Channel
 
 .SH ENVIRONMENT
 .TP
-.I CHANSRV_LOG_LEVEL
-Logging level for chansrv. Case-insensitive. Takes one of these string values:
-.RS 8
-.TP
-.B LOG_LEVEL_ALWAYS
-Only log essential messages
-.TP
-.B LOG_LEVEL_ERROR
-Log errors and above
-.TP
-.B LOG_LEVEL_WARNING
-Log warnings and above
-.TP
-.B LOG_LEVEL_INFO
-Log info and above. This is the default if nothing is specified
-.TP
-.B LOG_LEVEL_DEBUG
-Log debug and above
-.TP
-.B LOG_LEVEL_TRACE
-Log everything
-.TP
-.RE
-.TP
 .I CHANSRV_LOG_PATH
 Path to the location where the log file is stored. If not specified,
 $\fBXDG_DATA_HOME/xrdp\fP or \fB$HOME/.local/share/xrdp\fP is used instead.

docs/man/xrdp.ini.5.in

@@ -243,6 +243,18 @@ If set to \fB1\fR, \fBtrue\fR or \fByes\fR this option enables logging to syslog
 \fBSyslogLevel\fR=\fIlevel\fR
 This option sets the logging level for syslog. It can have the same values of \fBLogLevel\fR. If \fBSyslogLevel\fR is greater than \fBLogLevel\fR, its value is lowered to that of \fBLogLevel\fR.
 
+.TP
+\fBEnableConsole\fR=\fI[true|false]\fR
+If set to \fB1\fR, \fBtrue\fR or \fByes\fR, this option enables logging to the console (ie. stdout).
+
+.TP
+\fBConsoleLevel\fR=\fIlevel\fR
+Logging level for the console. It can have the same values as \fBLogLevel\fR. Defaults to \fBDEBUG\fR.
+
+.TP
+\fBEnableProcessId\fR=\fI[true|false]\fR
+If set to \fB1\fR, \fBtrue\fR or \fByes\fR, this option enables logging the process id in all log messages. Defaults to \fBfalse\fR.
+
 .SH "CHANNELS"
 The Remote Desktop Protocol supports several channels, which are used to transfer additional data like sound, clipboard data and others.
 Channel names not listed here will be blocked by \fBxrdp\fP.

sesman/access.c

@@ -48,7 +48,7 @@ access_login_allowed(const char *user)
 
     if ((0 == g_cfg->sec.ts_users_enable) && (0==g_cfg->sec.ts_always_group_check))
     {
-        LOG_DBG("Terminal Server Users group is disabled, allowing authentication");
+        log_message(LOG_LEVEL_INFO, "Terminal Server Users group is disabled, allowing authentication");
         return 1;
     }
 
@@ -96,7 +96,7 @@ access_login_mng_allowed(const char *user)
 
     if (0 == g_cfg->sec.ts_admins_enable)
     {
-        LOG_DBG("[MNG] Terminal Server Admin group is disabled, "
+        log_message(LOG_LEVEL_INFO, "[MNG] Terminal Server Admin group is disabled, "
                 "allowing authentication");
         return 1;
     }
@@ -109,7 +109,7 @@ access_login_mng_allowed(const char *user)
 
     if (g_cfg->sec.ts_admins == gid)
     {
-        LOG_DBG("[MNG] ts_users is user's primary group");
+        LOG(LOG_LEVEL_INFO, "[MNG] ts_users is user's primary group");
         return 1;
     }
 

sesman/chansrv/Makefile.am

@@ -68,7 +68,6 @@ xrdp_chansrv_SOURCES = \
   fifo.h \
   irp.c \
   irp.h \
-  mlog.h \
   rail.c \
   rail.h \
   smartcard.c \