Quieter logging

PostSRSd will be quieter unless instructed otherwise by the new debug=on configuration option. Fixes #149
roehling · Jul 14, 2023 · 9187684 · 9187684
1 parent aa6e664
commit 9187684
Show file tree

Hide file tree

Showing 7 changed files with 71 additions and 18 deletions.
diff --git a/data/postsrsd.conf.in b/data/postsrsd.conf.in
@@ -176,3 +176,9 @@ chroot-dir = "@POSTSRSD_CHROOTDIR@"
 # will also send all messages to the syslog mail facility.
 #
 syslog = off
+
+# Debug
+# This option makes PostSRSd more verbose in its logging, which can be useful
+# to hunt down configuration problems.
+#
+debug = off
diff --git a/src/config.c b/src/config.c
@@ -163,6 +163,7 @@ cfg_t* config_from_commandline(int argc, char* const* argv)
         CFG_STR("chroot-dir", DEFAULT_CHROOT_DIR, CFGF_NONE),
         CFG_BOOL("daemonize", cfg_false, CFGF_NONE),
         CFG_BOOL("syslog", cfg_false, CFGF_NONE),
+        CFG_BOOL("debug", cfg_false, CFGF_NONE),
         CFG_END(),
     };
     cfg_t* cfg = cfg_init(opts, CFGF_NONE);
@@ -282,6 +283,12 @@ srs_t* srs_from_config(cfg_t* cfg)
             return NULL;
         }
     }
+    if (srs->numsecrets == 0 || srs->secrets == NULL || srs->secrets[0] == NULL)
+    {
+        log_error("need at least one secret");
+        srs_free(srs);
+        return NULL;
+    }
     char* faketime = getenv("POSTSRSD_FAKETIME");
     if (faketime)
     {

diff --git a/src/main.c b/src/main.c
@@ -189,6 +189,7 @@ static void handle_socketmap_client(cfg_t* cfg, srs_t* srs,
         {
             netstring_write(fp_write, "PERM Invalid query.", 19);
             fflush(fp_write);
+            log_error("invalid socketmap query, closing connection");
             break;
         }
         alarm(0);
@@ -197,12 +198,14 @@ static void handle_socketmap_client(cfg_t* cfg, srs_t* srs,
         {
             netstring_write(fp_write, "PERM Invalid query.", 19);
             fflush(fp_write);
+            log_error("invalid socketmap query, closing connection");
             break;
         }
         if (len > 512 + (size_t)(addr - request))
         {
             netstring_write(fp_write, "PERM Too big.", 13);
             fflush(fp_write);
+            log_warn("socketmap query is too big");
             continue;
         }
         char* rewritten = NULL;
@@ -220,6 +223,7 @@ static void handle_socketmap_client(cfg_t* cfg, srs_t* srs,
         {
             error = true;
             info = "Invalid map.";
+            log_warn("invalid key in socketmap query");
         }
         if (rewritten)
         {
@@ -269,6 +273,8 @@ int main(int argc, char** argv)
         goto shutdown;
     if (cfg_getbool(cfg, "syslog"))
         log_enable_syslog();
+    if (cfg_getbool(cfg, "debug"))
+        log_set_verbosity(LogDebug);
     srs = srs_from_config(cfg);
     if (!srs)
         goto shutdown;

diff --git a/src/srs.c b/src/srs.c
@@ -35,15 +35,15 @@ char* postsrsd_forward(const char* addr, const char* domain, srs_t* srs,
     {
         if (info)
             *info = "No domain.";
-        log_info("<%s> not rewritten: no domain", addr);
+        log_debug("<%s> not rewritten: no domain", addr);
         return NULL;
     }
     const char* input_domain = at + 1;
     if (domain_set_contains(local_domains, input_domain))
     {
         if (info)
             *info = "Need not rewrite local domain.";
-        log_info("<%s> not rewritten: local domain", addr);
+        log_debug("<%s> not rewritten: local domain", addr);
         return NULL;
     }
     char db_alias_buf[35];
@@ -102,7 +102,14 @@ char* postsrsd_reverse(const char* addr, srs_t* srs, database_t* db,
     {
         if (info)
             *info = srs_strerror(result);
-        log_info("<%s> not reversed: %s", addr, srs_strerror(result));
+        if (result != SRS_ENOTSRSADDRESS)
+        {
+            log_info("<%s> not reversed: %s", addr, srs_strerror(result));
+        }
+        else
+        {
+            log_debug("<%s> not reversed: %s", addr, srs_strerror(result));
+        }
         return NULL;
     }
     const char* at = strchr(buffer, '@');
@@ -138,7 +145,7 @@ char* postsrsd_reverse(const char* addr, srs_t* srs, database_t* db,
         }
         else
         {
-            log_info("<%s> not reversed: no database for alias", addr);
+            log_warn("<%s> not reversed: no database for alias", addr);
             if (error)
                 *error = true;
             if (info)

diff --git a/src/util.c b/src/util.c
@@ -469,23 +469,18 @@ char* endpoint_for_redis(const char* s, int* port)
     return strndup(s, colon - s);
 }
 
-enum priority
-{
-    LogDebug,
-    LogInfo,
-    LogWarn,
-    LogError,
-};
-
+static enum log_priority log_prio = LogInfo;
 static const char* priority_labels[] = {"debug: ", "", "warn: ", "error: "};
 
 #ifdef HAVE_SYSLOG_H
 static bool use_syslog = false;
 static int syslog_priorities[] = {LOG_DEBUG, LOG_INFO, LOG_WARNING, LOG_ERR};
 #endif
 
-static void vlog(enum priority prio, const char* fmt, va_list ap)
+static void vlog(enum log_priority prio, const char* fmt, va_list ap)
 {
+    if (prio < log_prio)
+        return;
     char buffer[1088];
     size_t prefix_len =
         snprintf(buffer, sizeof(buffer), "postsrsd: %s", priority_labels[prio]);
@@ -515,6 +510,19 @@ void log_enable_syslog()
 #endif
 }
 
+void log_set_verbosity(enum log_priority prio)
+{
+    log_prio = prio;
+}
+
+void log_debug(const char* fmt, ...)
+{
+    va_list ap;
+    va_start(ap, fmt);
+    vlog(LogDebug, fmt, ap);
+    va_end(ap);
+}
+
 void log_info(const char* fmt, ...)
 {
     va_list ap;

diff --git a/src/util.h b/src/util.h
@@ -22,6 +22,12 @@
 
 #define MAYBE_UNUSED(x) (void)(x)
 
+#ifdef __GNUC__
+#    define ATTRIBUTE(x) __attribute__((x))
+#else
+#    define ATTRIBUTE(x)
+#endif
+
 struct domain_set;
 typedef struct domain_set domain_set_t;
 struct list;
@@ -59,12 +65,21 @@ void list_destroy(list_t* L, list_deleter_t deleter);
 char* endpoint_for_milter(const char* s);
 char* endpoint_for_redis(const char* s, int* port);
 
+enum log_priority
+{
+    LogDebug,
+    LogInfo,
+    LogWarn,
+    LogError,
+};
+
 void log_enable_syslog();
-void log_debug(const char* fmt, ...);
-void log_info(const char* fmt, ...);
-void log_warn(const char* fmt, ...);
-void log_error(const char* fmt, ...);
+void log_set_verbosity(enum log_priority prio);
+void log_debug(const char* fmt, ...) ATTRIBUTE(format(printf, 1, 2));
+void log_info(const char* fmt, ...) ATTRIBUTE(format(printf, 1, 2));
+void log_warn(const char* fmt, ...) ATTRIBUTE(format(printf, 1, 2));
+void log_error(const char* fmt, ...) ATTRIBUTE(format(printf, 1, 2));
 void log_perror(int errno, const char* prefix);
-void log_fatal(const char* fmt, ...) __attribute__((noreturn));
+void log_fatal(const char* fmt, ...) ATTRIBUTE(noreturn);
 
 #endif
diff --git a/tests/unit/CMakeLists.txt b/tests/unit/CMakeLists.txt
@@ -19,6 +19,10 @@ macro(add_postsrsd_test name)
     target_include_directories(
         ${name}_executable PRIVATE ${CMAKE_BINARY_DIR} ${CMAKE_SOURCE_DIR}/src
     )
+    target_compile_definitions(
+        ${name}_executable PRIVATE _GNU_SOURCE _POSIX_C_SOURCE
+                                   _FILE_OFFSET_BITS=64
+    )
     target_link_libraries(${name}_executable PRIVATE Check::check)
     target_compile_features(${name}_executable PRIVATE c_std_99)
     if(TESTS_WITH_ASAN)