log_pgsql: deprecate the module and rename its build flag

The PostgreSQL logging target sees little use and has been a source of
memory safety bugs. Mark it as deprecated and scheduled for removal, but
give anyone depending on it a chance to object first.

To make sure this is not missed, the LOG_PGSQL build flag now fails the
build with an explanation, building the module requires the renamed
LOG_PGSQL_DEPRECATED flag, cmake prints a warning when it is used, and the
module logs a deprecation warning on startup.

7 files changed, 41 insertions, 5 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index cbce9ec7..3f6bdeec 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,13 @@
 
 ## Unreleased
 
+### Deprecations
+- `log_pgsql` is deprecated and scheduled for removal. The `LOG_PGSQL` build
+  flag now fails the build; build with `LOG_PGSQL_DEPRECATED=TRUE` to keep it
+  for now, and the module logs a warning on startup. If you depend on it,
+  please object at https://github.com/accel-ppp/accel-ppp/issues, otherwise it
+  will be deleted.
+
 ### Features
 - New `metrics` module: HTTP endpoint exposing the same numbers as `accel-cmd show stat` at `/metrics`, in either Prometheus exposition or JSON format. Configurable listen address and optional IPv4 CIDR allow-list.
 
diff --git a/README.md b/README.md
index a78cb145..dd210758 100644
--- a/README.md
+++ b/README.md
@@ -48,7 +48,7 @@ monitoring kernel modules. Optional features require their corresponding
 development libraries:
 
 * Net-SNMP for NETSNMP=TRUE
-* PostgreSQL client libraries for LOG_PGSQL=TRUE
+* PostgreSQL client libraries for LOG_PGSQL_DEPRECATED=TRUE
 * Lua for LUA=TRUE or a specific Lua version such as LUA=5.3
 
 
@@ -74,7 +74,10 @@ Useful build options:
 * RADIUS=FALSE omits RADIUS support.
 * SHAPER=FALSE omits the traffic-shaping module.
 * NETSNMP=TRUE builds SNMP support.
-* LOG_PGSQL=TRUE builds PostgreSQL logging support.
+* LOG_PGSQL_DEPRECATED=TRUE builds PostgreSQL logging support. The module is
+  deprecated and scheduled for removal, the old LOG_PGSQL flag now fails the
+  build. If you depend on it, please say so at
+  https://github.com/accel-ppp/accel-ppp/issues.
 
 For example, to build the IPoE and VLAN monitoring modules for the running
 kernel:
diff --git a/accel-pppd/accel-ppp.conf b/accel-pppd/accel-ppp.conf
index 56fe0a8c..3fefcf27 100644
--- a/accel-pppd/accel-ppp.conf
+++ b/accel-pppd/accel-ppp.conf
@@ -2,7 +2,7 @@
 log_file
 #log_syslog
 #log_tcp
-#log_pgsql
+#log_pgsql (deprecated, scheduled for removal)
 
 connlimit
 
diff --git a/accel-pppd/accel-ppp.conf.5 b/accel-pppd/accel-ppp.conf.5
index bcddfed4..eee33bf0 100644
--- a/accel-pppd/accel-ppp.conf.5
+++ b/accel-pppd/accel-ppp.conf.5
@@ -36,6 +36,7 @@ This is logging target which logs messages over TCP/IP.
 .TP
 .BI log_pgsql
 This is logging target which logs messages to PostgreSQL.
+Deprecated and scheduled for removal, see the [log-pgsql] section.
 .TP
 .BI pptp
 .br
@@ -1337,6 +1338,10 @@ log all messages including debug messages
 .SH [log-pgsql]
 .br
 Configuration of log_pgsql module.
+.br
+This module is deprecated and scheduled for removal. It is built only when
+accel-ppp is configured with LOG_PGSQL_DEPRECATED=TRUE. If you depend on it,
+please object at https://github.com/accel-ppp/accel-ppp/issues.
 .TP
 .BI "conninfo=" conninfo
 Conninfo to connect to PostgreSQL server.
diff --git a/accel-pppd/logs/CMakeLists.txt b/accel-pppd/logs/CMakeLists.txt
index 4d8a631a..0052e1ae 100644
--- a/accel-pppd/logs/CMakeLists.txt
+++ b/accel-pppd/logs/CMakeLists.txt
@@ -33,7 +33,15 @@ IF(LOG_SYSLOG)
 	)
 ENDIF(LOG_SYSLOG)
 
+# log_pgsql is deprecated and scheduled for removal, the build flag was
+# renamed so that nobody keeps building it without noticing
 IF(LOG_PGSQL)
+	message(FATAL_ERROR "${Esc}[31mLOG_PGSQL is deprecated.\n The log_pgsql module is scheduled for removal in a future release.\n Build with -DLOG_PGSQL_DEPRECATED=TRUE if you still need it, and please say so at https://github.com/accel-ppp/accel-ppp/issues so that it is kept${Esc}[m")
+ENDIF(LOG_PGSQL)
+
+IF(LOG_PGSQL_DEPRECATED)
+	message(WARNING "${Esc}[33mBuilding deprecated module log_pgsql, it is scheduled for removal in a future release.\n Please say so at https://github.com/accel-ppp/accel-ppp/issues if you need it kept${Esc}[m")
+
 	find_library(PGSQL_LIBRARY pq
 					HINTS ${CMAKE_SOURCE_DIR}/lib/linux/gcc/${BIT}/lib
 					PATHS ${CMAKE_SOURCE_DIR}/lib/linux/gcc/${BIT}/lib
@@ -47,5 +55,5 @@ IF(LOG_PGSQL)
 	INSTALL(TARGETS log_pgsql
 		LIBRARY DESTINATION lib${LIB_SUFFIX}/accel-ppp
 	)
-ENDIF(LOG_PGSQL)
+ENDIF(LOG_PGSQL_DEPRECATED)
 
diff --git a/accel-pppd/logs/log_pgsql.c b/accel-pppd/logs/log_pgsql.c
index 5b3fd6ae..6589da0e 100644
--- a/accel-pppd/logs/log_pgsql.c
+++ b/accel-pppd/logs/log_pgsql.c
@@ -1,3 +1,13 @@
+/*
+ * DEPRECATED
+ *
+ * This module is scheduled for removal in a future release. It is built
+ * only when the deprecated LOG_PGSQL_DEPRECATED build flag is given.
+ * If you depend on it, please object at
+ * https://github.com/accel-ppp/accel-ppp/issues, otherwise it will be
+ * deleted.
+ */
+
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -293,6 +303,9 @@ static void init(void)
 {
 	char *opt;
 
+	log_warn("log_pgsql: this module is deprecated and is scheduled for removal,"
+		 " please object at https://github.com/accel-ppp/accel-ppp/issues if you need it\n");
+
 	spinlock_init(&queue_lock);
 
 	opt = conf_get_opt("log-pgsql", "conninfo");
diff --git a/contrib/gentoo/net-dialup/accel-ppp/accel-ppp-9999.ebuild b/contrib/gentoo/net-dialup/accel-ppp/accel-ppp-9999.ebuild
index 02bf282f..3edee26f 100644
--- a/contrib/gentoo/net-dialup/accel-ppp/accel-ppp-9999.ebuild
+++ b/contrib/gentoo/net-dialup/accel-ppp/accel-ppp-9999.ebuild
@@ -48,7 +48,7 @@ src_configure() {
 	fi
 
 	if  use postgres; then
-		mycmakeargs+=( "-DLOG_PGSQL=TRUE" )
+		mycmakeargs+=( "-DLOG_PGSQL_DEPRECATED=TRUE" )
 	fi
 	
 	if use shaper; then