diff --git a/bbs/auth.c b/bbs/auth.c index 4fbedf1..3ea8fd5 100644 --- a/bbs/auth.c +++ b/bbs/auth.c @@ -35,6 +35,7 @@ #include "include/crypt.h" #include "include/startup.h" #include "include/cli.h" +#include "include/callback.h" /*! \note Even though multiple auth providers are technically allowed, in general only 1 should be registered. * The original thinking behind allowing multiple is to allow alternates for authentication @@ -54,116 +55,56 @@ struct auth_provider { static RWLIST_HEAD_STATIC(providers, auth_provider); -static int (*registerprovider)(struct bbs_node *node) = NULL; -static void *registermod = NULL; +/* Unlike auth providers, there is only 1 user registration handler */ +BBS_SINGULAR_CALLBACK_DECLARE(registerprovider, int, struct bbs_node *node); -static int (*pwresethandler)(const char *username, const char *password) = NULL; -static void *pwresetmod = NULL; +/* Only one password reset handler */ +BBS_SINGULAR_CALLBACK_DECLARE(pwresethandler, int, const char *username, const char *password); -static struct bbs_user* (*userinfohandler)(const char *username) = NULL; -static void *userinfomod = NULL; +/* Only one user info handler */ +BBS_SINGULAR_CALLBACK_DECLARE(userinfohandler, struct bbs_user *, const char *username); -static struct bbs_user** (*userlisthandler)(void) = NULL; -static void *userlistmod = NULL; +/* Only one user list handler */ +BBS_SINGULAR_CALLBACK_DECLARE(userlisthandler, struct bbs_user**, void); int __bbs_register_user_registration_provider(int (*regprovider)(struct bbs_node *node), void *mod) { - /* Unlike auth providers, there is only 1 user registration handler */ - if (registerprovider) { - bbs_error("A user registration provider is already registered.\n"); - return -1; - } - - registerprovider = regprovider; - registermod = mod; - return 0; + return bbs_singular_callback_register(®isterprovider, regprovider, mod); } int bbs_unregister_user_registration_provider(int (*regprovider)(struct bbs_node *node)) { - if (regprovider != registerprovider) { - bbs_error("User registration provider does not match registered provider\n"); - return -1; - } - - registerprovider = NULL; - registermod = NULL; - return 0; + return bbs_singular_callback_unregister(®isterprovider, regprovider); } int __bbs_register_password_reset_handler(int (*handler)(const char *username, const char *password), void *mod) { - /* Only one password reset handler */ - if (pwresethandler) { - bbs_error("A password reset handler is already registered.\n"); - return -1; - } - - pwresethandler = handler; - pwresetmod = mod; - return 0; + return bbs_singular_callback_register(&pwresethandler, handler, mod); } int bbs_unregister_password_reset_handler(int (*handler)(const char *username, const char *password)) { - if (handler != pwresethandler) { - bbs_error("Password reset handler does not match registered handler\n"); - return -1; - } - - pwresethandler = NULL; - pwresetmod = NULL; - return 0; + return bbs_singular_callback_unregister(&pwresethandler, handler); } int __bbs_register_user_info_handler(struct bbs_user* (*handler)(const char *username), void *mod) { - /* Only one user info handler */ - if (userinfohandler) { - bbs_error("A user info handler is already registered.\n"); - return -1; - } - - userinfohandler = handler; - userinfomod = mod; - return 0; + return bbs_singular_callback_register(&userinfohandler, handler, mod); } int bbs_unregister_user_info_handler(struct bbs_user* (*handler)(const char *username)) { - if (handler != userinfohandler) { - bbs_error("User info handler does not match registered handler\n"); - return -1; - } - - userinfohandler = NULL; - userinfomod = NULL; - return 0; + return bbs_singular_callback_unregister(&userinfohandler, handler); } int __bbs_register_user_list_handler(struct bbs_user** (*handler)(void), void *mod) { - /* Only one user list handler */ - if (userlisthandler) { - bbs_error("A user list handler is already registered.\n"); - return -1; - } - - userlisthandler = handler; - userlistmod = mod; - return 0; + return bbs_singular_callback_register(&userlisthandler, handler, mod); } int bbs_unregister_user_list_handler(struct bbs_user** (*handler)(void)) { - if (handler != userlisthandler) { - bbs_error("User list handler does not match registered handler\n"); - return -1; - } - - userlisthandler = NULL; - userlistmod = NULL; - return 0; + return bbs_singular_callback_unregister(&userlisthandler, handler); } int __bbs_register_auth_provider(const char *name, int (*provider)(AUTH_PROVIDER_PARAMS), void *mod) @@ -219,7 +160,7 @@ int bbs_num_auth_providers(void) } RWLIST_UNLOCK(&providers); - if (!registerprovider) { + if (!bbs_singular_callback_registered(®isterprovider)) { /* This is kind of kludgy way to check this to warn, * but that way we don't need to expose this separately to bbs.c, * since it calls this function to ensure we have auth providers already. @@ -704,16 +645,14 @@ int bbs_user_register(struct bbs_node *node) { int res; - if (!registerprovider) { + if (bbs_singular_callback_execute_pre(®isterprovider)) { bbs_error("No user registration provider is currently registered, registration rejected\n"); return -1; } node->menu = "Register"; - bbs_assert_exists(registermod); - bbs_module_ref(registermod, 2); - res = registerprovider(node); - bbs_module_unref(registermod, 2); + res = BBS_SINGULAR_CALLBACK_EXECUTE(registerprovider)(node); + bbs_singular_callback_execute_post(®isterprovider); node->menu = NULL; if (bbs_user_is_registered(node->user)) { /* we might have returned -1 after registration succeeded on timeout */ @@ -732,15 +671,13 @@ int bbs_user_reset_password(const char *username, const char *password) { int res; - if (!pwresethandler) { + if (bbs_singular_callback_execute_pre(&pwresethandler)) { bbs_error("No password reset handler is currently registered\n"); return -1; } - bbs_assert_exists(pwresetmod); - bbs_module_ref(pwresetmod, 3); - res = pwresethandler(username, password); - bbs_module_unref(pwresetmod, 3); + res = BBS_SINGULAR_CALLBACK_EXECUTE(pwresethandler)(username, password); + bbs_singular_callback_execute_post(&pwresethandler); if (!res) { struct bbs_event event; @@ -759,15 +696,13 @@ struct bbs_user *bbs_user_info_by_username(const char *username) { struct bbs_user *user = NULL; - if (!userinfohandler) { + if (bbs_singular_callback_execute_pre(&userinfohandler)) { bbs_error("No user info handler is currently registered\n"); return NULL; } - bbs_assert_exists(userinfomod); - bbs_module_ref(userinfomod, 4); - user = userinfohandler(username); - bbs_module_unref(userinfomod, 4); + user = BBS_SINGULAR_CALLBACK_EXECUTE(userinfohandler)(username); + bbs_singular_callback_execute_post(&userinfohandler); return user; } @@ -776,15 +711,13 @@ struct bbs_user **bbs_user_list(void) { struct bbs_user **userlist = NULL; - if (!userlisthandler) { + if (bbs_singular_callback_execute_pre(&userlisthandler)) { bbs_error("No user list handler is currently registered\n"); return NULL; } - bbs_assert_exists(userlistmod); - bbs_module_ref(userlistmod, 5); - userlist = userlisthandler(); - bbs_module_unref(userlistmod, 5); + userlist = BBS_SINGULAR_CALLBACK_EXECUTE(userlisthandler)(); + bbs_singular_callback_execute_post(&userlisthandler); return userlist; } diff --git a/bbs/callback.c b/bbs/callback.c new file mode 100644 index 0000000..3d8a72e --- /dev/null +++ b/bbs/callback.c @@ -0,0 +1,135 @@ +/* + * LBBS -- The Lightweight Bulletin Board System + * + * Copyright (C) 2024, Naveen Albert + * + * Naveen Albert + * + * This program is free software, distributed under the terms of + * the GNU General Public License Version 2. See the LICENSE file + * at the top of the source tree. + */ + +/*! \file + * + * \brief Module Callbacks + * + * \author Naveen Albert + */ + +#include "include/bbs.h" + +#include "include/callback.h" +#include "include/module.h" + +static inline void set_function_pointer(struct bbs_singular_callback *scb, void *ptr) +{ + /* We can't directly assign to *scb->func_pointer_ptr, since it's a void pointer, + * and we can't dereference void pointers in C. + * Therefore, cast to some arbitrary but concrete type, so we're allowed + * to make the assignment. + * This works, since all pointers are the same size. */ + int **ptraddress = scb->func_pointer_ptr; + *ptraddress = (int*) ptr; +} + +static inline int function_pointer_exists(struct bbs_singular_callback *scb) +{ + int **ptraddress = scb->func_pointer_ptr; + return *ptraddress ? 1 : 0; +} + +/*! \brief Dereference a void pointer. Yeah, you read that right. */ +static inline void *function_pointer_value(struct bbs_singular_callback *scb) +{ + int **ptraddress = scb->func_pointer_ptr; + return (void*) *ptraddress; +} + +int bbs_singular_callback_destroy(struct bbs_singular_callback *scb) +{ + pthread_rwlock_destroy(&scb->lock); + return 0; +} + +int __bbs_singular_callback_register(struct bbs_singular_callback *scb, void *cbptr, void *mod, const char *file, int line, const char *func) +{ + pthread_rwlock_wrlock(&scb->lock); + + if (function_pointer_exists(scb)) { + /* Already a callback function registered. */ + __bbs_log(LOG_ERROR, 0, file, line, func, "Could not register callback function %p (already one registered)\n", cbptr); + pthread_rwlock_unlock(&scb->lock); + return -1; + } + + /* Yes, it may seem a bit dangerous that we're using void for a function pointer, + * but the idea is that we're being called by a function that accepts that function + * as an argument, so it's already passed type checking in that case. */ + set_function_pointer(scb, cbptr); + scb->mod = mod; + + pthread_rwlock_unlock(&scb->lock); + return 0; +} + +int __bbs_singular_callback_unregister(struct bbs_singular_callback *scb, void *cbptr, const char *file, int line, const char *func) +{ + pthread_rwlock_wrlock(&scb->lock); + if (cbptr != function_pointer_value(scb)) { + __bbs_log(LOG_ERROR, 0, file, line, func, "Can't unregister callback function %p (not the one registered)\n", cbptr); + pthread_rwlock_unlock(&scb->lock); + return -1; + } + + /* Locking is needed essentially to prevent TOCTOU (Time of Check, Time of Use) bugs. + * Without locking, one thread could check if the callback exists, but that callback + * could be unregistered before it actually executes it using the (now NULL) pointer. */ + set_function_pointer(scb, NULL); + scb->mod = NULL; + + pthread_rwlock_unlock(&scb->lock); + return 0; +} + +int bbs_singular_callback_registered(struct bbs_singular_callback *scb) +{ + return function_pointer_exists(scb); +} + +int __bbs_singular_callback_execute_pre(struct bbs_singular_callback *scb, void *refmod, const char *file, int line, const char *func) +{ + pthread_rwlock_rdlock(&scb->lock); + if (!function_pointer_exists(scb)) { + pthread_rwlock_unlock(&scb->lock); + return -1; /* No callback registered */ + } + if (scb->mod) { + __bbs_module_ref(scb->mod, 100, refmod, file, line, func); + } + + /* We don't actually execute the callback here, because + * the callbacks allowed by this API are completely arbitrary, + * and could have an arbitrary return type and number (and type) of parameters. + * We'll let the caller do that. */ + + /* We intentionally return without unlocking scb->lock, + * since the call to bbs_singular_callback_execute_post will unlock. + * If scb->mod could never be NULL, we could technically unlock here, + * because the callback cannot be unregistered by the module providing it + * as long as that module is in use (has a positive refcount). + * However, to support the bbs_singular_callback API within the core, + * we just keep holding the read lock. Doesn't hurt anything, since we + * we wouldn't be able to unregister anyways while this is held, + * and a read lock won't inhibit concurrent invocations of the callback. */ + return 0; +} + +int __bbs_singular_callback_execute_post(struct bbs_singular_callback *scb, void *refmod, const char *file, int line, const char *func) +{ + if (scb->mod) { + __bbs_module_unref(scb->mod, 100, refmod, file, line, func); + } + pthread_rwlock_unlock(&scb->lock); + return 0; +} diff --git a/bbs/kvs.c b/bbs/kvs.c index 31a69e7..560b1c5 100644 --- a/bbs/kvs.c +++ b/bbs/kvs.c @@ -19,38 +19,21 @@ #include "include/bbs.h" -#include "include/module.h" #include "include/kvs.h" +#include "include/callback.h" /* At this time, there is only support for 1 KVS backend loaded at a time. Because why would we need more? */ -static struct kvs_callbacks *callbacks = NULL; -static void *kvsmod = NULL; +BBS_SINGULAR_STRUCT_CALLBACK_DECLARE(callbacks, kvs_callbacks); int __bbs_register_kvs_backend(struct kvs_callbacks *cb, int priority, void *mod) { - /* Unlike auth providers, there is only 1 user registration handler */ - if (callbacks) { - bbs_error("A KVS backend is already registered.\n"); - return -1; - } - UNUSED(priority); - - callbacks = cb; - kvsmod = mod; - return 0; + return bbs_singular_callback_register(&callbacks, cb, mod); } int bbs_unregister_kvs_backend(struct kvs_callbacks *cb) { - if (cb != callbacks) { - bbs_error("KVS provider does not match registered provider\n"); - return -1; - } - - callbacks = NULL; - kvsmod = NULL; - return 0; + return bbs_singular_callback_unregister(&callbacks, cb); } int bbs_kvs_get(const char *key, size_t keylen, char *buf, size_t len, size_t *outlen) @@ -60,14 +43,14 @@ int bbs_kvs_get(const char *key, size_t keylen, char *buf, size_t len, size_t *o ptr = outlen ? outlen : &outlentmp; *ptr = 0; - if (!callbacks) { + + if (bbs_singular_callback_execute_pre(&callbacks)) { bbs_error("No KVS backend currently registered\n"); return -1; } - bbs_module_ref(kvsmod, 1); - res = callbacks->get(key, keylen, buf, len, NULL, ptr); - bbs_module_unref(kvsmod, 1); + res = BBS_SINGULAR_STRUCT_CALLBACK_EXECUTE(callbacks)->get(key, keylen, buf, len, NULL, ptr); + bbs_singular_callback_execute_post(&callbacks); bbs_debug(6, "KVS GET(%s) => %lu bytes\n", key, *ptr); return res; @@ -81,14 +64,13 @@ char *bbs_kvs_get_allocated(const char *key, size_t keylen, size_t *outlen) ptr = outlen ? outlen : &outlentmp; *ptr = 0; - if (!callbacks) { + if (bbs_singular_callback_execute_pre(&callbacks)) { bbs_error("No KVS backend currently registered\n"); return NULL; } - bbs_module_ref(kvsmod, 2); - res = callbacks->get(key, keylen, NULL, 0, &c, ptr); - bbs_module_unref(kvsmod, 2); + res = BBS_SINGULAR_STRUCT_CALLBACK_EXECUTE(callbacks)->get(key, keylen, NULL, 0, &c, ptr); + bbs_singular_callback_execute_post(&callbacks); if (res) { return NULL; @@ -102,14 +84,13 @@ int bbs_kvs_put(const char *key, size_t keylen, const char *value, size_t valuel { int res; - if (!callbacks) { + if (bbs_singular_callback_execute_pre(&callbacks)) { bbs_error("No KVS backend currently registered\n"); return -1; } - bbs_module_ref(kvsmod, 3); - res = callbacks->put(key, keylen, value, valuelen); - bbs_module_unref(kvsmod, 3); + res = BBS_SINGULAR_STRUCT_CALLBACK_EXECUTE(callbacks)->put(key, keylen, value, valuelen); + bbs_singular_callback_execute_post(&callbacks); bbs_debug(6, "KVS PUT(%s) => %lu bytes\n", key, valuelen); return res; @@ -119,14 +100,13 @@ int bbs_kvs_del(const char *key, size_t keylen) { int res; - if (!callbacks) { + if (bbs_singular_callback_execute_pre(&callbacks)) { bbs_error("No KVS backend currently registered\n"); return -1; } - bbs_module_ref(kvsmod, 4); - res = callbacks->del(key, keylen); - bbs_module_unref(kvsmod, 4); + res = BBS_SINGULAR_STRUCT_CALLBACK_EXECUTE(callbacks)->del(key, keylen); + bbs_singular_callback_execute_post(&callbacks); bbs_debug(6, "KVS DEL(%s)\n", key); return res; diff --git a/include/callback.h b/include/callback.h new file mode 100644 index 0000000..fdc0642 --- /dev/null +++ b/include/callback.h @@ -0,0 +1,143 @@ +/* + * LBBS -- The Lightweight Bulletin Board System + * + * Copyright (C) 2024, Naveen Albert + * + * Naveen Albert + * + */ + +/*! \file + * + * \brief Module Callbacks + * + */ + +/* Needed for pthread_rwlock_t */ +#include + +struct bbs_singular_callback { + pthread_rwlock_t lock; /*!< Read/write lock for callback function */ + void *mod; /*!< Module that registered the callback */ + /* It's temping to just use void * as the type for the callback function, + * and store it here - and this works for all usage of this API itself. + * However, this poses a problem when trying to execute the callback, + * and we also want type checking for the right number (and correct type) + * of arguments. We could cast the void pointer back to the type + * of the function, but that's rather messy to use in practice. + * Therefore, we actually store the callback function pointer + * in the calling source file, and store the pointer to the pointer in this struct, + * but make this indirection transparent to callers using some macros. */ + void *func_pointer_ptr; /*!< Callback function */ + unsigned int initialized:1; +}; + +/*! \note We cannot use memset with the above struct, to avoid overwriting scb->func_pointer_ptr, + * which is already set at this point, thanks to BBS_SINGULAR_CALLBACK_DECLARE. + * Thus, we explicitly initialize each member in the below macro. + * + * To make the interface as simple as possible, we statically initialize the rwlock. + */ + +/* == Function variants == */ + +/*! + * \brief Declare a singularly provided callback function and its interface + * \param name Unique variable name + * \param returntype The return type of the callback function + * \param ... Variadic arguments for each callback function parameter + */ +#define BBS_SINGULAR_CALLBACK_DECLARE(name, returntype, ...) \ + static returntype (*__ ## name ## _cb_func)(__VA_ARGS__) = NULL; \ + static struct bbs_singular_callback name = { \ + .func_pointer_ptr = &__ ## name ## _cb_func, \ + .mod = NULL, \ + .lock = PTHREAD_RWLOCK_INITIALIZER \ + }; + +/*! + * \brief Get the pointer to the callback function + * \param name Same name provided to BBS_SINGULAR_CALLBACK_DECLARE + * \return Function pointer + */ +#define BBS_SINGULAR_CALLBACK_EXECUTE(name) __ ## name ## _cb_func + +/* == Struct variants (struct pointer, instead of function pointer (the dereferenced struct contains function pointers)) == */ +/* Since we're storing the function pointer as void, we can just store the struct pointer there instead */ + +#define BBS_SINGULAR_STRUCT_CALLBACK_DECLARE(name, structtype) \ + static struct structtype (*__ ## name ## _cb_struct) = NULL; \ + static struct bbs_singular_callback name = { \ + .func_pointer_ptr = &__ ## name ## _cb_struct, \ + .mod = NULL, \ + .lock = PTHREAD_RWLOCK_INITIALIZER \ + }; + +/*! + * \brief Get the pointer to the struct of callbacks + * \param name Same name provided to BBS_SINGULAR_STRUCT_CALLBACK_DECLARE + * \return Function pointer + */ +#define BBS_SINGULAR_STRUCT_CALLBACK_EXECUTE(name) __ ## name ## _cb_struct + +/*! + * \brief Destroy a bbs_singular_callback + * \param scb + * \retval 0 on success, -1 on failure + * \note There is no corresponding initialize function, since initialization is static. + * To be "correct", this function SHOULD be called, particularly from dynamic modules, + * but this interface was designed such that not calling this from core files won't matter. + */ +int bbs_singular_callback_destroy(struct bbs_singular_callback *scb) __attribute__((nonnull (1))); + +int __bbs_singular_callback_register(struct bbs_singular_callback *scb, void *cbptr, void *mod, const char *file, int line, const char *func) __attribute__((nonnull (1,2)));; + +/*! + * \brief Register a singularly provided callback + * \param scb + * \param cbptr Callback function + * \param mod Providing module + * \retval 0 if callback registered + * \retval -1 callback not registered (already another callback registered) + * \warning Because this API uses a void pointer to allow any function pointer, the function prototype + * MUST be validated by the calling function, to prevent ABI hanky panky. + */ +#define bbs_singular_callback_register(scb, cbptr, mod) __bbs_singular_callback_register(scb, cbptr, mod, __FILE__, __LINE__, __func__) + +int __bbs_singular_callback_unregister(struct bbs_singular_callback *scb, void *cbptr, const char *file, int line, const char *func) __attribute__((nonnull (1,2))); + +/*! + * \brief Unregister a singularly provided callback + * \param scb + * \param cbptr Callback function to unregister + */ +#define bbs_singular_callback_unregister(scb, cbptr) __bbs_singular_callback_unregister(scb, cbptr, __FILE__, __LINE__, __func__) + +/*! + * \brief Check whether a callback function is registered + * \param scb + * \retval 1 if callback function currently registered + * \retval 0 if no callback function currently registered + */ +int bbs_singular_callback_registered(struct bbs_singular_callback *scb); + +int __bbs_singular_callback_execute_pre(struct bbs_singular_callback *scb, void *refmod, const char *file, int line, const char *func); + +/*! + * \param Begin executing a singularly provided callback function + * \param scb + * \retval 0 on success (okay to execute callback function) + * \retval -1 on failure + * \warning UNDER NO CIRCUMSTANCES should the callback function be executed if this function returns failure + */ +#define bbs_singular_callback_execute_pre(scb) __bbs_singular_callback_execute_pre(scb, BBS_MODULE_SELF, __FILE__, __LINE__, __func__) + +int __bbs_singular_callback_execute_post(struct bbs_singular_callback *scb, void *refmod, const char *file, int line, const char *func); + +/*! + * \param Stop executing a singularly provided callback function + * \param scb + * \retval 0 on success, -1 on failure + * \warning This function MUST be called after finish execution of the callback + */ +#define bbs_singular_callback_execute_post(scb) __bbs_singular_callback_execute_post(scb, BBS_MODULE_SELF, __FILE__, __LINE__, __func__) diff --git a/modules/mod_http.c b/modules/mod_http.c index 92fd54b..f2bb6bf 100644 --- a/modules/mod_http.c +++ b/modules/mod_http.c @@ -51,6 +51,7 @@ #include "include/crypt.h" #include "include/event.h" #include "include/cli.h" +#include "include/callback.h" #include "include/mod_http.h" @@ -131,8 +132,8 @@ static RWLIST_HEAD_STATIC(listeners, http_listener); static RWLIST_HEAD_STATIC(routes, http_route); static RWLIST_HEAD_STATIC(sessions, session); -static enum http_response_code (*proxy_handler)(struct http_session *http) = NULL; -static void *proxy_handler_mod = NULL; +BBS_SINGULAR_CALLBACK_DECLARE(proxy_handler, enum http_response_code, struct http_session *http); +/* Extra information for the callback that isn't handled by the regular singular callback interface: */ static unsigned short int proxy_port = 0; static enum http_method proxy_methods = HTTP_METHOD_UNDEF; @@ -1762,11 +1763,10 @@ static int http_handle_request(struct http_session *http, char *buf) } else if (!(proxy_methods & http->req->method)) { bbs_debug(3, "Proxy handler does not support %s\n", http_method_name(http->req->method)); } else { - if (proxy_handler) { + if (!bbs_singular_callback_execute_pre(&proxy_handler)) { bbs_debug(4, "Passing %s proxy request for %s to proxy handler\n", http_method_name(http->req->method), http->req->uri); - bbs_module_ref(proxy_handler_mod, 1); - code = proxy_handler(http); - bbs_module_unref(proxy_handler_mod, 1); + code = BBS_SINGULAR_CALLBACK_EXECUTE(proxy_handler)(http); + bbs_singular_callback_execute_post(&proxy_handler); return code; } bbs_event_dispatch(http->node, EVENT_NODE_BAD_REQUEST); /* Likely spam traffic. */ @@ -2829,28 +2829,23 @@ int http_unregister_route(enum http_response_code (*handler)(struct http_session int __http_register_proxy_handler(unsigned short int port, enum http_method methods, enum http_response_code (*handler)(struct http_session *http), void *mod) { - if (proxy_handler) { - bbs_error("Proxy handler already registered\n"); - return -1; + int res = bbs_singular_callback_register(&proxy_handler, handler, mod); + if (!res) { + /* Not 100% perfect, since we're no longer holding the lock, but probably fine */ + proxy_port = port; + proxy_methods = methods; } - proxy_handler_mod = mod; - proxy_handler = handler; - proxy_port = port; - proxy_methods = methods; - return 0; + return res; } int http_unregister_proxy_handler(enum http_response_code (*handler)(struct http_session *http)) { - if (handler != proxy_handler) { - bbs_error("Proxy handler %p not currently registered\n", handler); - return -1; + int res = bbs_singular_callback_unregister(&proxy_handler, handler); + if (!res) { + proxy_port = 0; + proxy_methods = HTTP_METHOD_UNDEF; } - proxy_handler = NULL; - proxy_handler_mod = NULL; - proxy_port = 0; - proxy_methods = HTTP_METHOD_UNDEF; - return 0; + return res; } static int cli_http_routes(struct bbs_cli_args *a) @@ -2889,6 +2884,7 @@ static int unload_module(void) /* Remove any lingering sessions */ RWLIST_WRLOCK_REMOVE_ALL(&sessions, entry, session_free); bbs_cli_unregister_multiple(cli_commands_http); + bbs_singular_callback_destroy(&proxy_handler); return 0; } diff --git a/modules/mod_mail.c b/modules/mod_mail.c index caf1d59..23af9f8 100644 --- a/modules/mod_mail.c +++ b/modules/mod_mail.c @@ -41,6 +41,7 @@ #include "include/stringlist.h" #include "include/range.h" #include "include/cli.h" +#include "include/callback.h" #include "include/mod_mail.h" @@ -360,65 +361,59 @@ int mailbox_has_activity(struct mailbox *mbox) return res; } -static int (*sieve_validate)(const char *filename, struct mailbox *mbox, char **errormsg) = NULL; -static char *sieve_capabilities = NULL; -static void *sievemod = NULL; +BBS_SINGULAR_CALLBACK_DECLARE(sieve_validate, int, const char *filename, struct mailbox *mbox, char **errormsg); +static char *sieve_capabilities = NULL; /* Additional callback data not handled by singular callback interface */ int __sieve_register_provider(int (*validate)(const char *filename, struct mailbox *mbox, char **errormsg), char *capabilities, void *mod) { + int res; if (strlen_zero(capabilities)) { bbs_error("Missing capabilities\n"); return -1; - } else if (sieve_validate) { - bbs_error("A Sieve implementation is already registered.\n"); + } + res = bbs_singular_callback_register(&sieve_validate, validate, mod); + if (!res) { + sieve_capabilities = capabilities; /* Steal the reference */ + } else { free(capabilities); - return -1; } - - sieve_capabilities = capabilities; /* Steal the reference */ - sieve_validate = validate; - sievemod = mod; - return 0; + return res; } int sieve_unregister_provider(int (*validate)(const char *filename, struct mailbox *mbox, char **errormsg)) { - if (sieve_validate != validate) { - bbs_error("Sieve implementation %p does not match registered provider %p\n", validate, sieve_validate); - return -1; + int res = bbs_singular_callback_unregister(&sieve_validate, validate); + if (!res) { + FREE(sieve_capabilities); } - - sieve_validate = NULL; - sievemod = NULL; - free(sieve_capabilities); - sieve_capabilities = NULL; - return 0; + return res; } char *sieve_get_capabilities(void) { char *caps; - /* Technically not safe to just return directly, since the module could go away while we're using it? */ - if (!sieve_capabilities) { + + /* Technically not safe to just return directly, since the module could go away while we're using it? + * So duplicate it and return that. */ + + if (bbs_singular_callback_execute_pre(&sieve_validate)) { bbs_error("No Sieve implementation is currently registered\n"); return NULL; } - bbs_module_ref(sievemod, 2); caps = strdup(sieve_capabilities); - bbs_module_unref(sievemod, 2); + bbs_singular_callback_execute_post(&sieve_validate); return caps; } int sieve_validate_script(const char *filename, struct mailbox *mbox, char **errormsg) { int res; - if (!sieve_validate) { + if (bbs_singular_callback_execute_pre(&sieve_validate)) { bbs_error("No Sieve implementation is currently registered\n"); return -1; } - bbs_module_ref(sievemod, 3); - res = sieve_validate(filename, mbox, errormsg); - bbs_module_unref(sievemod, 3); + res = BBS_SINGULAR_CALLBACK_EXECUTE(sieve_validate)(filename, mbox, errormsg); + bbs_singular_callback_execute_post(&sieve_validate); return res; } @@ -2112,6 +2107,7 @@ static int unload_module(void) { bbs_cli_unregister_multiple(cli_commands_mailboxes); mailbox_cleanup(); + bbs_singular_callback_destroy(&sieve_validate); return 0; } diff --git a/nets/net_smtp.c b/nets/net_smtp.c index 4fed615..cd44f87 100644 --- a/nets/net_smtp.c +++ b/nets/net_smtp.c @@ -62,6 +62,7 @@ #include "include/test.h" #include "include/mail.h" #include "include/cli.h" +#include "include/callback.h" #include "include/mod_mail.h" #include "include/net_smtp.h" @@ -577,6 +578,7 @@ static int handle_helo(struct smtp_session *smtp, char *s, int ehlo) smtp_reply0_nostatus(smtp, 250, "PIPELINING"); smtp_reply0_nostatus(smtp, 250, "SIZE %u", max_message_size); /* RFC 1870 */ smtp_reply0_nostatus(smtp, 250, "8BITMIME"); /* RFC 6152 */ + smtp_reply0_nostatus(smtp, 250, "ETRN"); /* RFC 1985 */ if (!smtp->secure && ssl_available()) { smtp_reply0_nostatus(smtp, 250, "STARTTLS"); } @@ -738,40 +740,16 @@ int smtp_unregister_delivery_agent(struct smtp_delivery_agent *agent) return h ? 0 : -1; } -/*! \todo FIXME Any time we have a single callback (not a list like delivery agents, with its own R/W list lock), - * we need a rwlock for the callback. - * Most such callbacks in the BBS don't have this. - * Might be worth creating an abstraction (e.g. bbs_module_callback) to simplify this. */ -static pthread_rwlock_t smtp_queue_processor_lock; -static int (*smtp_queue_processor)(struct smtp_session *smtp, const char *cmd, const char *args) = NULL; -static void *smtp_queue_processor_mod = NULL; +BBS_SINGULAR_CALLBACK_DECLARE(smtp_queue_processor, int, struct smtp_session *smtp, const char *cmd, const char *args); int __smtp_register_queue_processor(int (*queue_processor)(struct smtp_session *smtp, const char *cmd, const char *args), void *mod) { - pthread_rwlock_wrlock(&smtp_queue_processor_lock); - if (smtp_queue_processor) { - pthread_rwlock_unlock(&smtp_queue_processor_lock); - bbs_error("SMTP On Demand Mail Relay already registered\n"); - return -1; - } - smtp_queue_processor = queue_processor; - smtp_queue_processor_mod = mod; - pthread_rwlock_unlock(&smtp_queue_processor_lock); - return 0; + return bbs_singular_callback_register(&smtp_queue_processor, queue_processor, mod); } int smtp_unregister_queue_processor(int (*queue_processor)(struct smtp_session *smtp, const char *cmd, const char *args)) { - pthread_rwlock_wrlock(&smtp_queue_processor_lock); - if (smtp_queue_processor != queue_processor) { - pthread_rwlock_unlock(&smtp_queue_processor_lock); - bbs_error("SMTP On Demand Relay %p not registered\n", queue_processor); - return -1; - } - smtp_queue_processor_mod = NULL; - smtp_queue_processor = NULL; - pthread_rwlock_unlock(&smtp_queue_processor_lock); - return 0; + return bbs_singular_callback_unregister(&smtp_queue_processor, queue_processor); } /*! \brief Parse parameter data in MAIL FROM */ @@ -858,21 +836,13 @@ static int handle_etrn(struct smtp_session *smtp, char *s) return 0; } - pthread_rwlock_rdlock(&smtp_queue_processor_lock); - if (!smtp_queue_processor) { + if (bbs_singular_callback_execute_pre(&smtp_queue_processor)) { /* No queue processor registered, so ETRN not supported */ - pthread_rwlock_unlock(&smtp_queue_processor_lock); smtp_reply(smtp, 501, 5.5.4, "Unrecognized parameter"); return 0; } - bbs_module_ref(smtp_queue_processor_mod, 7); - /* Technically, as soon as we ref the module, - * we are safe to unlock, since the module can no longer - * be unloaded until it's unreffed. However, it does - * no harm to hold a read lock longer, either. */ - res = smtp_queue_processor(smtp, "ETRN", args); - bbs_module_unref(smtp_queue_processor_mod, 7); - pthread_rwlock_unlock(&smtp_queue_processor_lock); + res = BBS_SINGULAR_CALLBACK_EXECUTE(smtp_queue_processor)(smtp, "ETRN", args); + bbs_singular_callback_execute_post(&smtp_queue_processor); switch (res) { case 250: @@ -2934,6 +2904,7 @@ static int load_module(void) /* If we can't start the TCP listeners, decline to load */ if (bbs_start_tcp_listener3(smtp_enabled ? smtp_port : 0, smtps_enabled ? smtps_port : 0, msa_enabled ? msa_port : 0, "SMTP", "SMTPS", "SMTP (MSA)", __smtp_handler)) { + bbs_singular_callback_destroy(&smtp_queue_processor); goto cleanup; } @@ -2973,6 +2944,7 @@ static int unload_module(void) if (smtplogfp) { fclose(smtplogfp); } + bbs_singular_callback_destroy(&smtp_queue_processor); return 0; }