From f269f6d1d5dee177251cd2468b76a90ff677a4d5 Mon Sep 17 00:00:00 2001 From: Harald Welte Date: Wed, 20 Mar 2019 11:39:40 +0100 Subject: make use of OTC_GLOBAL when allocating library-internal contexts As libosmcore is now managing the global talloc contexts, there's no point in having APIs where the user tells the library about which talloc contexts to use for a given sub-system. However, completely ignoring the talloc contexts passed in by existing applications would unfortunately break LeakSanitizer. It appears to track if any dynamically allocated pointeres are not stored anywhere, and that would be exactly the situation created here. So the new behavior is: Use the context passed in by an application, and fall back to the library-internal OTC_GLOBAL if none is passed in. Change-Id: I48f475efd3ee0d5120b8fc30861e852d1a6920b1 --- include/osmocom/core/signal.h | 4 +++- src/counter.c | 8 ++++++-- src/ctrl/control_vty.c | 7 ++++++- src/gb/gprs_bssgp.c | 3 +++ src/gsm/lapd_core.c | 4 +++- src/gsm/lapdm.c | 2 +- src/logging.c | 4 ++-- src/signal.c | 22 ++++++++++++++-------- src/stats.c | 4 ++-- src/vty/telnet_interface.c | 5 ++--- tests/ctrl/ctrl_test.c | 7 ++++--- tests/vty/vty_test.c | 2 +- 12 files changed, 47 insertions(+), 25 deletions(-) diff --git a/include/osmocom/core/signal.h b/include/osmocom/core/signal.h index 449b9762..0f178439 100644 --- a/include/osmocom/core/signal.h +++ b/include/osmocom/core/signal.h @@ -1,6 +1,7 @@ #pragma once #include +#include /*! \defgroup signal Intra-application signals * @{ @@ -34,7 +35,8 @@ typedef int osmo_signal_cbfn(unsigned int subsys, unsigned int signal, void *han /* Management */ -void *osmo_signal_talloc_ctx_init(void *root_ctx); +void *osmo_signal_talloc_ctx_init(void *root_ctx) + OSMO_DEPRECATED("libosmocore internally allocates this context now."); int osmo_signal_register_handler(unsigned int subsys, osmo_signal_cbfn *cbfn, void *data); void osmo_signal_unregister_handler(unsigned int subsys, osmo_signal_cbfn *cbfn, void *data); diff --git a/src/counter.c b/src/counter.c index 0fa31661..482dfc29 100644 --- a/src/counter.c +++ b/src/counter.c @@ -1,7 +1,7 @@ /*! \file counter.c * utility routines for keeping some statistics. */ /* - * (C) 2009 by Harald Welte + * (C) 2009,2019 by Harald Welte * * All Rights Reserved * @@ -39,8 +39,12 @@ void *tall_ctr_ctx; * \returns Allocated counter on success; NULL on error */ struct osmo_counter *osmo_counter_alloc(const char *name) { - struct osmo_counter *ctr = talloc_zero(tall_ctr_ctx, struct osmo_counter); + struct osmo_counter *ctr; + + if (!tall_ctr_ctx) + tall_ctr_ctx = talloc_named_const(OTC_GLOBAL, 0, "osmo_counter"); + ctr = talloc_zero(tall_ctr_ctx, struct osmo_counter); if (!ctr) return NULL; diff --git a/src/ctrl/control_vty.c b/src/ctrl/control_vty.c index e141a4c8..2961983e 100644 --- a/src/ctrl/control_vty.c +++ b/src/ctrl/control_vty.c @@ -79,9 +79,14 @@ static int config_write_ctrl(struct vty *vty) return CMD_SUCCESS; } +/*! Initialize the VTY configuration for the CTRL interface. + * \param[in] ctx should be NULL; only used for legacy compatibility + * \returns 0 on success; negative on error */ int ctrl_vty_init(void *ctx) { - ctrl_vty_ctx = ctx; + ctrl_vty_ctx = talloc_named_const(ctx ? ctx : OTC_GLOBAL, 0, "ctrl-vty"); + if (!ctrl_vty_ctx) + return -1; install_lib_element(CONFIG_NODE, &cfg_ctrl_cmd); install_node(&ctrl_node, config_write_ctrl); diff --git a/src/gb/gprs_bssgp.c b/src/gb/gprs_bssgp.c index 926b0efa..47b764c7 100644 --- a/src/gb/gprs_bssgp.c +++ b/src/gb/gprs_bssgp.c @@ -173,6 +173,9 @@ struct bssgp_bvc_ctx *btsctx_alloc(uint16_t bvci, uint16_t nsei) { struct bssgp_bvc_ctx *ctx; + if (!bssgp_tall_ctx) + bssgp_tall_ctx = talloc_named_const(OTC_GLOBAL, 0, "bssgp"); + ctx = talloc_zero(bssgp_tall_ctx, struct bssgp_bvc_ctx); if (!ctx) return NULL; diff --git a/src/gsm/lapd_core.c b/src/gsm/lapd_core.c index e0c232fe..9e9c0c6a 100644 --- a/src/gsm/lapd_core.c +++ b/src/gsm/lapd_core.c @@ -254,7 +254,7 @@ static void lapd_dl_newstate(struct lapd_datalink *dl, uint32_t state) dl->state = state; } -void *tall_lapd_ctx = NULL; +__thread void *tall_lapd_ctx; /*! Initialize LAPD datalink instance and allocate history * \param[in] dl caller-allocated datalink structure @@ -315,6 +315,8 @@ void lapd_dl_init2(struct lapd_datalink *dl, uint8_t k, uint8_t v_range, int max lapd_dl_newstate(dl, LAPD_STATE_IDLE); + if (!tall_lapd_ctx) + tall_lapd_ctx = talloc_named_const(OTC_GLOBAL, 1, "lapd context"); dl->tx_hist = talloc_zero_array(tall_lapd_ctx, struct lapd_history, dl->range_hist); } diff --git a/src/gsm/lapdm.c b/src/gsm/lapdm.c index 8620cabe..8101a797 100644 --- a/src/gsm/lapdm.c +++ b/src/gsm/lapdm.c @@ -126,7 +126,7 @@ const struct value_string osmo_ph_prim_names[] = { { 0, NULL } }; -extern void *tall_lapd_ctx; +extern __thread void *tall_lapd_ctx; static int lapdm_send_ph_data_req(struct lapd_msg_ctx *lctx, struct msgb *msg); static int send_rslms_dlsap(struct osmo_dlsap_prim *dp, diff --git a/src/logging.c b/src/logging.c index a40008e9..60530688 100644 --- a/src/logging.c +++ b/src/logging.c @@ -1076,7 +1076,7 @@ int log_targets_reopen(void) /*! Initialize the Osmocom logging core * \param[in] inf Information regarding logging categories, could be NULL - * \param[in] ctx talloc context for logging allocations + * \param[in] ctx should be NULL; only used for egacy API compatibility * \returns 0 in case of success, negative in case of error * * If inf is NULL then only library-internal categories are initialized. @@ -1089,7 +1089,7 @@ int log_init(const struct log_info *inf, void *ctx) /* Ensure that log_init is not called multiple times */ OSMO_ASSERT(tall_log_ctx == NULL) - tall_log_ctx = talloc_named_const(ctx, 1, "logging"); + tall_log_ctx = talloc_named_const(ctx ? ctx : OTC_GLOBAL, 1, "logging"); if (!tall_log_ctx) return -ENOMEM; diff --git a/src/signal.c b/src/signal.c index be3b7778..04014d65 100644 --- a/src/signal.c +++ b/src/signal.c @@ -36,7 +36,11 @@ * \file signal.c */ +/* legacy export for old openbsc.git versions that insist on setting this directly */ void *tall_sigh_ctx; +/* actual talloc context used these days */ +static void *tall_sigh_ctx2; + static LLIST_HEAD(signal_handler_list); struct signal_handler { @@ -46,14 +50,11 @@ struct signal_handler { void *data; }; -/*! Initialize a signal_handler talloc context for \ref osmo_signal_register_handler. - * Create a talloc context called "osmo_signal". - * \param[in] root_ctx talloc context used as parent for the new "osmo_signal" ctx. - * \returns the new osmo_signal talloc context, e.g. for reporting - */ +/* DEPRECATED: Never really worked and we now have OTC_GLOBAL */ void *osmo_signal_talloc_ctx_init(void *root_ctx) { - tall_sigh_ctx = talloc_named_const(root_ctx, 0, "osmo_signal"); - return tall_sigh_ctx; + if (!tall_sigh_ctx2) + tall_sigh_ctx2 = talloc_named_const(root_ctx ? root_ctx : OTC_GLOBAL, 0, "osmo_signal"); + return tall_sigh_ctx2; } /*! Register a new signal handler @@ -67,7 +68,12 @@ int osmo_signal_register_handler(unsigned int subsys, { struct signal_handler *sig_data; - sig_data = talloc_zero(tall_sigh_ctx, struct signal_handler); + if (!tall_sigh_ctx2) + tall_sigh_ctx2 = talloc_named_const(OTC_GLOBAL, 0, "osmo_signal"); + if (!tall_sigh_ctx2) + return -ENOMEM; + + sig_data = talloc_zero(tall_sigh_ctx2, struct signal_handler); if (!sig_data) return -ENOMEM; diff --git a/src/stats.c b/src/stats.c index c91a9780..5c98809e 100644 --- a/src/stats.c +++ b/src/stats.c @@ -226,10 +226,10 @@ void osmo_stats_reporter_free(struct osmo_stats_reporter *srep) } /*! Initilize the stats reporting module; call this once in your program - * \param[in] ctx Talloc context from which stats related memory is allocated */ + * \param[in] ctx should be NULL; only for legacy API compatibility */ void osmo_stats_init(void *ctx) { - osmo_stats_ctx = ctx; + osmo_stats_ctx = talloc_named_const(ctx ? ctx : OTC_GLOBAL, 0, "stats"); osmo_stat_item_discard_all(¤t_stat_item_index); is_initialised = 1; diff --git a/src/vty/telnet_interface.c b/src/vty/telnet_interface.c index cd32e68f..20d616c8 100644 --- a/src/vty/telnet_interface.c +++ b/src/vty/telnet_interface.c @@ -75,7 +75,7 @@ int telnet_init(void *tall_ctx, void *priv, int port) } /*! Initialize telnet based VTY interface - * \param[in] tall_ctx \ref talloc context + * \param[in] tall_ctx should be NULL; nly for legacy API compatibility * \param[in] priv private data to be passed to callback * \param[in] ip IP to listen to ('::1' for localhost, '::0' for all, ...) * \param[in] port TCP port number to bind to @@ -84,8 +84,7 @@ int telnet_init_dynif(void *tall_ctx, void *priv, const char *ip, int port) { int rc; - tall_telnet_ctx = talloc_named_const(tall_ctx, 1, - "telnet_connection"); + tall_telnet_ctx = talloc_named_const(tall_ctx ? tall_ctx : OTC_GLOBAL, 0, "telnet_connection"); rc = osmo_sock_init_ofd( &server_socket, diff --git a/tests/ctrl/ctrl_test.c b/tests/ctrl/ctrl_test.c index b46e9ac5..ff48038a 100644 --- a/tests/ctrl/ctrl_test.c +++ b/tests/ctrl/ctrl_test.c @@ -4,6 +4,7 @@ #include #include +#include #include #include #include @@ -459,7 +460,7 @@ static struct log_info info = { int main(int argc, char **argv) { - ctx = talloc_named_const(NULL, 1, "ctrl_test"); + ctx = OTC_GLOBAL; osmo_init_logging2(ctx, &info); msgb_talloc_ctx_init(ctx, 0); @@ -478,8 +479,8 @@ int main(int argc, char **argv) test_deferred_cmd(); - /* Expecting root ctx + msgb root ctx + 5 logging elements */ - if (talloc_total_blocks(ctx) != 7) { + /* Expecting root ctx + name + select + msgb root ctx + 5 logging elements */ + if (talloc_total_blocks(ctx) != 9) { talloc_report_full(ctx, stdout); OSMO_ASSERT(false); } diff --git a/tests/vty/vty_test.c b/tests/vty/vty_test.c index 1db0d5ce..df07730a 100644 --- a/tests/vty/vty_test.c +++ b/tests/vty/vty_test.c @@ -597,7 +597,7 @@ int main(int argc, char **argv) test_numeric_range(); /* Leak check */ - OSMO_ASSERT(talloc_total_blocks(stats_ctx) == 1); + OSMO_ASSERT(talloc_total_blocks(stats_ctx) == 2); printf("All tests passed\n"); -- cgit v1.2.3