build: Fix compilation with -Og and other debugging improvements.

* Fix compilation with -O0/-Og.
* Remove inline from functions that don't inline without optimization.
* Ignore warnings for variables that complain about uninitialized use
  without optimization.
* Improve debugging for various modules on failure / off-nominal cases.
* tcp_client: Use calling location for log messages.
* Add assertion API to simplify custom assertion handling.

.github/workflows/main.yml

@@ -2,7 +2,7 @@ name: CI
 
 on:
   push:
-    branches: [ master ]
+    branches: [ master, dev ]
   pull_request:
     branches: [ master ]
 
@@ -50,6 +50,22 @@ jobs:
           sudo tests/test -ddddddddd -DDDDDDDDDD -x
           sudo apt-get install -y valgrind
           sudo tests/test -ddddddddd -DDDDDDDDDD -ex
+  without-optimization:
+    runs-on: ubuntu-22.04
+    name: Ubuntu 22.04, without optimization
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4
+      - name: Build LBBS
+        run: |
+          sudo sed -i 's/azure\.//' /etc/apt/sources.list
+          sudo sed -i 's/-O3/-O0/' Makefile
+          sudo ./scripts/install_prereq.sh
+          sudo make modcheck
+          sudo make
+          sudo make install
+          sudo make samples
+          sudo make tests
   debian-12:
     runs-on: ubuntu-24.04
     name: Debian 12

bbs/io.c

@@ -113,9 +113,11 @@ int bbs_io_session_register(struct bbs_io_transformations *s, enum bbs_io_sessio
 int bbs_io_session_unregister(struct bbs_io_transformations *s)
 {
 	struct io_session *i;
+	int total = 0;
 
 	RWLIST_WRLOCK(&sessions);
 	RWLIST_TRAVERSE_SAFE_BEGIN(&sessions, i, entry) {
+		total++;
 		if (i->s == s) {
 			RWLIST_REMOVE_CURRENT(entry);
 			free(i);
@@ -125,7 +127,8 @@ int bbs_io_session_unregister(struct bbs_io_transformations *s)
 	RWLIST_TRAVERSE_SAFE_END;
 	RWLIST_UNLOCK(&sessions);
 	if (!i) {
-		bbs_warning("Transformation %p does not have an active session\n", s);
+		/* We traversed the entire list, so this count is accurate */
+		bbs_warning("Transformation %p does not have an active session (%d total)\n", s, total);
 	}
 	return i ? 0 : -1;
 }

bbs/node.c

@@ -1464,14 +1464,14 @@ static int authenticate(struct bbs_node *node)
 	return 0;
 }
 
-static inline long ns_since(struct timespec *start, struct timespec *now)
+static long ns_since(struct timespec *start, struct timespec *now)
 {
 	struct timespec diff;
 	timespecsub(now, start, &diff);
 	return (1000000000 * diff.tv_sec) + (diff.tv_nsec);
 }
 
-static inline int record_start_time(struct timespec *restrict start)
+static int record_start_time(struct timespec *restrict start)
 {
 	if (clock_gettime(CLOCK_MONOTONIC_RAW, start)) {
 		bbs_error("clock_gettime failed: %s\n", strerror(errno));
@@ -1737,7 +1737,7 @@ static int init_term_query_ansi_escape_support(struct bbs_node *node)
  * \retval 1 if positive cursor position query response received
  * \retval 0 Non-ANSI terminal (no positive response)
  */
-static inline int read_cursor_pos_response(struct bbs_node *node, struct timespec *restrict start, const char *restrict buf, int len)
+static int read_cursor_pos_response(struct bbs_node *node, struct timespec *restrict start, const char *restrict buf, int len)
 {
 	int i;
 	int res;
@@ -1800,7 +1800,7 @@ static inline int read_cursor_pos_response(struct bbs_node *node, struct timespe
 }
 
 /*! \brief Round to the nearest modem speed */
-static inline long int estimate_bps(long int bps)
+static long int estimate_bps(long int bps)
 {
 	if (bps < 75) {
 		return 45;

bbs/ratelimit.c

@@ -68,7 +68,7 @@ int bbs_rate_limit_init(struct bbs_rate_limit *r, int interval, int max)
 	return 0;
 }
 
-static inline long ms_since(struct timespec *start, struct timespec *now)
+static long ms_since(struct timespec *start, struct timespec *now)
 {
 	struct timespec diff;
 	timespecsub(now, start, &diff);

bbs/socket.c

@@ -2506,7 +2506,7 @@ ssize_t bbs_timed_write(int fd, const char *buf, size_t len, int ms)
 }
 
 /* \note Assumes fd is a file descriptor corresponding to a file */
-static inline off_t fd_get_filesize(int fd)
+static off_t fd_get_filesize(int fd)
 {
 	off_t max_offset, cur_offset;
 

bbs/tcp.c

@@ -99,20 +99,20 @@ ssize_t __attribute__ ((format (gnu_printf, 2, 3))) bbs_tcp_client_send(struct b
 	return res;
 }
 
-int bbs_tcp_client_expect(struct bbs_tcp_client *client, const char *delim, int attempts, int ms, const char *str)
+int __bbs_tcp_client_expect(struct bbs_tcp_client *client, const char *delim, int attempts, int ms, const char *str, const char *file, int line, const char *func)
 {
 	while (attempts-- > 0) {
 		ssize_t res = bbs_readline(client->rfd, &client->rldata, delim, ms);
 		if (res < 0) {
-			bbs_debug(3, "bbs_readline returned %ld\n", res);
+			__bbs_log(LOG_DEBUG, 3, file, line, func, "bbs_readline returned %ld\n", res);
 			bbs_readline_print_reset(&client->rldata);
 			return -1;
 		}
-		bbs_debug(7, "<= %s\n", client->buf);
+		__bbs_log(LOG_DEBUG, 7, file, line, func, "%p <= %s\n", client, client->buf);
 		if (strstr(client->buf, str)) {
 			return 0;
 		}
 	}
-	bbs_warning("Missing expected response (%s), got: %s\n", str, client->buf);
+	__bbs_log(LOG_WARNING, 0, file, line, func, "Missing expected response (%s), got: %s\n", str, client->buf);
 	return 1;
 }

bbs/variables.c

@@ -45,7 +45,7 @@ struct bbs_var {
 /* static RWLIST_HEAD_STATIC(global_vars, bbs_var); */
 static struct bbs_vars global_vars;
 
-static inline void bbs_var_destroy(struct bbs_var *var)
+static void bbs_var_destroy(struct bbs_var *var)
 {
 	free(var->value); /* Free variable */
 	free(var); /* Free key and the struct itself */

include/bbs.h

@@ -432,6 +432,11 @@ extern int option_dumpcore;
 #endif
 #define bbs_assert(x) __bbs_assert(x, #x, __FILE__, __LINE__, __func__)
 #define bbs_soft_assert(x) __bbs_soft_assert(x, #x, __FILE__, __LINE__, __func__)
+
+/*! \brief Same as bbs_soft_assert, but useful as part of an if condition to execute additional logic if the soft assertion fails.
+ * This avoids the need to do something like if (badcondition) { bbs_soft_assert(0); dosomething; } */
+#define bbs_assertion_failed(x) __bbs_soft_assertion_failed(x, #x, __FILE__, __LINE__, __func__)
+
 void __bbs_assert_nonfatal(const char *condition_str, const char *file, int line, const char *function);
 void __attribute__((noreturn)) __bbs_assert_fatal(const char *condition_str, const char *file, int line, const char *function);
 
@@ -450,6 +455,15 @@ static inline void __attribute__((always_inline)) __bbs_soft_assert(int conditio
 	}
 }
 
+static inline int __attribute__((always_inline)) __bbs_soft_assertion_failed(int condition, const char *condition_str, const char *file, int line, const char *function)
+{
+	if (__builtin_expect(!condition, 1)) {
+		__bbs_assert_nonfatal(condition_str, file, line, function);
+		return 1;
+	}
+	return 0;
+}
+
 #else
 /* Use builtin assert */
 #define bbs_assert(x) assert(x)

include/tcp.h

@@ -69,4 +69,6 @@ ssize_t bbs_tcp_client_send(struct bbs_tcp_client *client, const char *fmt, ...)
  * \param str Substring to expect
  * \retval 0 on success (substring contained in response), -1 on failure, 1 if max attempts reached
  */
-int bbs_tcp_client_expect(struct bbs_tcp_client *client, const char *delim, int attempts, int ms, const char *str);
+#define bbs_tcp_client_expect(client, delim, attempts, ms, str) __bbs_tcp_client_expect(client, delim, attempts, ms, str, __FILE__, __LINE__, __func__)
+
+int __bbs_tcp_client_expect(struct bbs_tcp_client *client, const char *delim, int attempts, int ms, const char *str, const char *file, int line, const char *func) __attribute__((nonnull (1,2, 5)));

io/io_compress.c

@@ -83,7 +83,7 @@ static ssize_t compress_and_send(struct compress_data *z, char *buf, size_t len)
 			return -1;
 		}
 		comp_len = sizeof(output) - z->compressor->avail_out;
-		bbs_debug(9, "Deflated to %lu bytes\n", comp_len);
+		bbs_debug(10, "Deflated to %lu bytes\n", comp_len);
 		wres = bbs_write(z->orig_wfd, output, comp_len);
 		if (wres <= 0) {
 			return -1;

io/io_tls.c

@@ -411,12 +411,16 @@ static void *ssl_io_thread(void *unused)
 					 * rather than waiting until ssl_fd_free. */
 					pfds[i].events = 0;
 					pfds[i].fd = -1; /* This does not trigger a POLLNVAL, negative fds are ignored by poll (see poll(2)) */
-					bbs_debug(7, "Skipping dead SSL read connection %p at index %d / %d\n", sfd->ssl, i, i / 2);
+					bbs_debug(7, "Skipping dead SSL read connection %p at index %d / %d (fd: %d/%d)\n", sfd->ssl, i, i / 2, sfd->fd, sfd->writepipe[0]);
 				} else {
 					pfds[i].fd = sfd->fd;
 					pfds[i].events = POLLIN | POLLPRI | POLLERR | POLLNVAL;
 				}
 				i++;
+				/*! \todo In some cases, we are still seeing an infinite loop of poll constantly returning activity on a dead connection.
+				 * I think this happens because even though we use -1 above, we still use the actual writepipe read fd here.
+				 * While we can't set this to -1 because of the comment below, in certain cases it may make sense to close this and use -1.
+				 * For now, more logging of the actual file descriptor number has been added so this theory can be confirmed. */
 				pfds[i].fd = sfd->writepipe[0];
 				/* If it's dead, we still need to read from the writepipe and discard,
 				 * or otherwise it might block a writing thread */
@@ -530,12 +534,12 @@ static void *ssl_io_thread(void *unused)
 			if (!inovertime && pfds[i].revents != POLLIN) { /* Something exceptional happened, probably something going away */
 				if (pfds[i].revents & (POLLNVAL | POLLHUP)) {
 					SSL *ssl = ssl_list[i / 2];
-					bbs_debug(5, "Skipping SSL at index %d / %d = %s\n", i, i/2, poll_revent_name(pfds[i].revents));
+					bbs_debug(5, "Skipping SSL %p at index %d / %d = %s (fd %d)\n", ssl, i, i/2, poll_revent_name(pfds[i].revents), pfds[i].fd);
 					MARK_DEAD(ssl);
 					needcreate = 1;
 					continue; /* Don't try to read(), that would fail */
 				} else {
-					bbs_debug(5, "SSL at index %d / %d = %s\n", i, i/2, poll_revent_name(pfds[i].revents));
+					bbs_debug(5, "SSL %p at index %d / %d = %s\n", ssl_list[i / 2], i, i/2, poll_revent_name(pfds[i].revents));
 				}
 			}
 			if (!inovertime && i == 0) {

modules/Makefile

@@ -28,7 +28,7 @@ $(DEPENDS):
 
 mod_mimeparse.o : mod_mimeparse.c mod_mimeparse.d
 	@echo "  [CC] $< -> $@"
-	$(CC) $(CFLAGS) -fPIC -DBBS_MODULE=\"$(basename $<)\" -DBBS_MODULE_SELF_SYM=__internal_$(basename $<)_self $(GMIME_FLAGS) -MMD -MP $(INC) -c $<
+	$(CC) $(CFLAGS) -fPIC -DBBS_MODULE=\"$(basename $<)\" -DBBS_MODULE_SELF_SYM=__internal_$(basename $<)_self $(GMIME_FLAGS) -Wno-inline -MMD -MP $(INC) -c $<
 
 mod_ncurses.o : mod_ncurses.c mod_ncurses.d
 	@echo "  [CC] $< -> $@"

modules/mod_asterisk_queues.c

@@ -240,7 +240,7 @@ static struct queue *find_queue(const char *name)
 static int update_queue_stats(void)
 {
 	int i;
-	struct queue *q, *lastq;
+	struct queue *q, *lastq = NULL; /* lastq can't be NULL, but gcc will complain maybe uninitialized with -Og */
 	int stale_queues = 0;
 	struct ami_response *resp;
 

modules/mod_discord.c

@@ -908,7 +908,7 @@ static void dump_user(struct bbs_node *node, int fd, const char *requsername, st
 	irc_relay_who_response(node, fd, "Discord", requsername, combined, unique, !(u->status == STATUS_IDLE || u->status == STATUS_OFFLINE));
 }
 
-static inline int discord_is_ready(void)
+static int discord_is_ready(void)
 {
 	time_t now, diff;
 

modules/mod_http.c

@@ -140,7 +140,7 @@ static enum http_method proxy_methods = HTTP_METHOD_UNDEF;
 /* == HTTP output I/O wrappers == */
 
 /*! \brief Actually write data to the HTTP client (potentially abstracted by TLS) */
-static inline ssize_t __http_direct_write(struct http_session *http, const char *buf, size_t len)
+static ssize_t __http_direct_write(struct http_session *http, const char *buf, size_t len)
 {
 	return bbs_node_fd_write(http->node, http->node->wfd, buf, len);
 }

modules/mod_mail.c

@@ -927,9 +927,8 @@ const char *mailbox_maildir(struct mailbox *mbox)
 size_t mailbox_maildir_len(struct mailbox *mbox)
 {
 #ifdef CHECK_MAILDIR_LEN_INTEGRITY
-	if (strlen(mbox->maildir) != mbox->maildirlen) {
+	if (bbs_assertion_failed(strlen(mbox->maildir) == mbox->maildirlen)) {
 		bbs_warning("maildir length mismatch: %lu != %lu\n", strlen(mbox->maildir), mbox->maildirlen);
-		bbs_soft_assert(0);
 	}
 #endif
 	return mbox->maildirlen;
@@ -943,9 +942,8 @@ int mailbox_maildir_validate(const char *maildir)
 	if (!dirlen) {
 		return 0;
 	}
-	if (dirlen <= rootmaildirlen) {
+	if (bbs_assertion_failed(dirlen > rootmaildirlen)) {
 		bbs_warning("maildir (%s) len (%lu) <= maildir root (%s) len (%lu)?\n", maildir, dirlen, mailbox_maildir(NULL), rootmaildirlen);
-		bbs_soft_assert(0);
 		return -1;
 	}
 	return 0;
@@ -1108,7 +1106,7 @@ unsigned int mailbox_get_next_uid(struct mailbox *mbox, struct bbs_node *node, c
 		/* Assume the file doesn't yet exist. */
 		uidfile_existed = bbs_file_exists(uidfile);
 		fp = fopen(uidfile, "a");
-		if (unlikely(!fp)) {
+		if (unlikely(!fp)) { /* Don't use bbs_assertion_failed, because we want to preserve errno */
 			bbs_error("fopen(%s) failed: %s\n", uidfile, strerror(errno));
 			bbs_soft_assert(0);
 		} else {

modules/mod_node_callbacks.c

@@ -89,7 +89,7 @@ static int interactive_start(struct bbs_node *node)
 	return 0;
 }
 
-static inline ssize_t print_birthday_banner(struct bbs_node *node)
+static ssize_t print_birthday_banner(struct bbs_node *node)
 {
 	return bbs_node_writef(node,
 		"\n"

modules/mod_slack.c

@@ -853,7 +853,7 @@ static void notify_unauthorized(const char *sender, const char *channel, const c
 	irc_relay_send_notice(sender, CHANNEL_USER_MODE_NONE, "Slack", sender, NULL, notice, NULL);
 }
 
-static inline void parse_parent_thread(struct slack_relay *relay, char *restrict ts, const char **restrict thread_ts, char const **restrict msg)
+static void parse_parent_thread(struct slack_relay *relay, char *restrict ts, const char **restrict thread_ts, char const **restrict msg)
 {
 	const char *word2;
 	long word1len, tsl;

modules/mod_smtp_delivery_external.c

@@ -680,9 +680,7 @@ static void smtp_trigger_dsn(enum smtp_delivery_action action, struct smtp_tx_da
 		return;
 	}
 
-	if (action != DELIVERY_FAILED && !notify_queue) {
-		return;
-	} else if (action == DELIVERY_DELIVERED) {
+	if (action == DELIVERY_DELIVERED) {
 		time_t sent;
 		if (!created) {
 			bbs_warning("Message has no sent time?\n");
@@ -693,6 +691,8 @@ static void smtp_trigger_dsn(enum smtp_delivery_action action, struct smtp_tx_da
 			return; /* Don't send reports on success UNLESS a message was previously delayed */
 		}
 		/* Do send a delivery report, it was likely previous queued and succeeded only on a retry */
+	} else if (action != DELIVERY_FAILED && !notify_queue) {
+		return;
 	}
 
 	tmp = strchr(error, ' ');
@@ -740,7 +740,7 @@ struct mailq_run {
 	bbs_mutex_t lock;
 };
 
-static inline void mailq_run_init(struct mailq_run *qrun, enum mailq_run_type type)
+static void mailq_run_init(struct mailq_run *qrun, enum mailq_run_type type)
 {
 	/* We could individually initialize each element in the struct,
 	 * but as the struct probably has no padding,
@@ -751,7 +751,7 @@ static inline void mailq_run_init(struct mailq_run *qrun, enum mailq_run_type ty
 	qrun->runstart = time(NULL);
 }
 
-static inline void mailq_run_cleanup(struct mailq_run *qrun)
+static void mailq_run_cleanup(struct mailq_run *qrun)
 {
 	bbs_mutex_destroy(&qrun->lock);
 }
@@ -1055,7 +1055,7 @@ static int __attribute__ ((nonnull (2, 4, 5, 6, 10))) try_mx_delivery(struct smt
  * \param retrycount Count of many times delivery has been attempted so far
  * \return Number of seconds that should pass from the last retry before we attempt delivery again
  */
-static inline time_t queue_retry_threshold(int retrycount)
+static time_t queue_retry_threshold(int retrycount)
 {
 	/* We use ~exponential backoff for queue retry timing,
 	 * as is generally recommended. */
@@ -1089,7 +1089,7 @@ static inline time_t queue_retry_threshold(int retrycount)
 	__builtin_unreachable();
 }
 
-static inline int skip_qfile(struct mailq_run *qrun, struct mailq_file *mqf)
+static int skip_qfile(struct mailq_run *qrun, struct mailq_file *mqf)
 {
 	/* This queue run may have filters applied to it */
 
@@ -1342,6 +1342,10 @@ static int run_queue(struct mailq_run *qrun, int (*queue_file_cb)(const char *di
 
 	bbs_rwlock_wrlock(&queue_lock);
 	queued_messages = bbs_dir_num_files(queue_dir);
+	if (!queued_messages) {
+		bbs_rwlock_unlock(&queue_lock);
+		return 0; /* If nothing in directory, nothing at all to process, return early */
+	}
 	bbs_debug(7, "Processing mail queue (%d message%s)\n", queued_messages, ESS(queued_messages));
 	/* If the number of queued messages is relatively small, we can just process them serially.
 	 * It's not worth the overhead of parallelization.