slab: Style guide says alloc functions should return the newly allocated structure...

https://wiki.freeradius.org/contributing/coding-standards#return-types_pointer

diff --git a/src/lib/util/slab.h b/src/lib/util/slab.h
index 3c2e07c62e7b64dc62dce6fb9c15143966d785e2..98266a783adcf6f7b8a6d84def8fba08b4724a8f 100644 (file)
--- a/src/lib/util/slab.h
+++ b/src/lib/util/slab.h
@@ -160,7 +160,6 @@ DIAG_OFF(unused-function) \
        /** Allocate a slab list to manage slabs of allocated memory \
         * \
         * @param[in] ctx               in which to allocate the slab list. \
-        * @param[out] out              slab_list that has been allocated. \
         * @param[in] el                Event list in which to run clean up function. \
         * @param[in] config            Slab config parameters. \
         * @param[in] alloc             Optional callback to use when allocating new elements. \
@@ -168,31 +167,40 @@ DIAG_OFF(unused-function) \
         * @param[in] uctx              to pass to callbacks. \
         * @param[in] release_reset     Should elements be reset and children freed when the element is released.\
         * @param[in] reserve_mru       If true, the most recently used element will be returned when an element is reserved. \
+        * @return \
+        *      - A new slab. \
+        *      - NULL on error. \
         */ \
-       static inline CC_HINT(nonnull(2)) int _name ## _slab_list_alloc(TALLOC_CTX *ctx, \
-                                                                              _name ## _slab_list_t **out, \
-                                                                              fr_event_list_t *el, \
-                                                                              fr_slab_config_t const *config, \
-                                                                              _type ## _slab_alloc_t alloc, \
-                                                                              _type ## _slab_reserve_t reserve, \
-                                                                              void *uctx, \
-                                                                              bool release_reset, \
-                                                                              bool reserve_mru) \
+       static inline _name ## _slab_list_t *_name ## _slab_list_alloc(TALLOC_CTX *ctx, \
+                                                                      fr_event_list_t *el, \
+                                                                      fr_slab_config_t const *config, \
+                                                                      _type ## _slab_alloc_t alloc, \
+                                                                      _type ## _slab_reserve_t reserve, \
+                                                                      void *uctx, \
+                                                                      bool release_reset, \
+                                                                      bool reserve_mru) \
        { \
-               MEM(*out = talloc_zero(ctx, _name ## _slab_list_t)); \
-               (*out)->el = el; \
-               (*out)->config = *config; \
-               if ((*out)->config.elements_per_slab == 0) \
-                       (*out)->config.elements_per_slab = (config->min_elements ? config->min_elements : 1); \
-               (*out)->alloc = alloc; \
-               (*out)->reserve = reserve; \
-               (*out)->uctx = uctx; \
-               (*out)->release_reset = release_reset; \
-               (*out)->reserve_mru = reserve_mru; \
-               _name ## _slab_init(&(*out)->reserved); \
-               _name ## _slab_init(&(*out)->avail); \
-               if (el) (void) fr_event_timer_in(*out, el, &(*out)->ev, config->interval, _ ## _name ## _slab_cleanup, *out); \
-               return 0; \
+               _name ## _slab_list_t *slab; \
+               MEM(slab = talloc_zero(ctx, _name ## _slab_list_t)); \
+               slab->el = el; \
+               slab->config = *config; \
+               if (slab->config.elements_per_slab == 0) { \
+                       slab->config.elements_per_slab = (config->min_elements ? config->min_elements : 1); \
+               } \
+               slab->alloc = alloc; \
+               slab->reserve = reserve; \
+               slab->uctx = uctx; \
+               slab->release_reset = release_reset; \
+               slab->reserve_mru = reserve_mru; \
+               _name ## _slab_init(&slab->reserved); \
+               _name ## _slab_init(&slab->avail); \
+               if (el) { \
+                       if (unlikely(fr_event_timer_in(slab, el, &slab->ev, config->interval, _ ## _name ## _slab_cleanup, slab) < 0)) { \
+                               talloc_free(slab); \
+                               return NULL; \
+                       }; \
+               } \
+               return slab; \
        } \
 \
        /** Callback for talloc freeing a slab element \

diff --git a/src/lib/util/slab_tests.c b/src/lib/util/slab_tests.c
index e6fc877e6715829540e445c62370e1c68b52e642..69be54714bba79cf370af91a1f533530b381caf4 100644 (file)
--- a/src/lib/util/slab_tests.c
+++ b/src/lib/util/slab_tests.c
@@ -57,13 +57,11 @@ static void test_alloc(void)
        test_slab_list_t        *test_slab_list;
        test_element_t          *test_elements[5];
        test_uctx_t             test_uctx, test_uctx2;
-       int                     ret = -1;
 
        /*
         *      Each slab will contain 2 elements, maximum of 4 elements allocated from slabs.
         */
-       ret = test_slab_list_alloc(NULL, &test_slab_list, NULL, &def_slab_config, NULL, NULL, NULL, true, false);
-       TEST_CHECK(ret == 0);
+       test_slab_list = test_slab_list_alloc(NULL, NULL, &def_slab_config, NULL, NULL, NULL, true, false);
        TEST_CHECK(test_slab_list != NULL);
        if (!test_slab_list) return;
 
@@ -129,15 +127,13 @@ static void test_alloc_fail(void)
 {
        test_slab_list_t        *test_slab_list;
        test_element_t          *test_elements[5];
-       int                     ret = -1;
        fr_slab_config_t        slab_config = def_slab_config;
 
        /*
         *      Each slab will contain 2 elements, maximum of 4 elements allocated from slabs.
         */
        slab_config.at_max_fail = true;
-       ret = test_slab_list_alloc(NULL, &test_slab_list, NULL, &slab_config, NULL, NULL, NULL, true, false);
-       TEST_CHECK(ret == 0);
+       test_slab_list = test_slab_list_alloc(NULL, NULL, &slab_config, NULL, NULL, NULL, true, false);
        TEST_CHECK(test_slab_list != NULL);
        if (!test_slab_list) return;
 
@@ -170,10 +166,8 @@ static void test_reuse_reset(void)
        test_slab_list_t        *test_slab_list;
        test_element_t          *test_elements[5];
        test_uctx_t             test_uctx;
-       int                     ret = -1;
 
-       ret = test_slab_list_alloc(NULL, &test_slab_list, NULL, &def_slab_config, NULL, NULL, NULL, true, false);
-       TEST_CHECK(ret == 0);
+       test_slab_list = test_slab_list_alloc(NULL, NULL, &def_slab_config, NULL, NULL, NULL, true, false);
        TEST_CHECK(test_slab_list != NULL);
        if (!test_slab_list) return;
 
@@ -232,10 +226,8 @@ static void test_reuse_noreset(void)
        test_slab_list_t        *test_slab_list;
        test_element_t          *test_elements[3];
        test_uctx_t             test_uctx;
-       int                     ret = -1;
 
-       ret = test_slab_list_alloc(NULL, &test_slab_list, NULL, &def_slab_config, NULL, NULL, NULL, false, false);
-       TEST_CHECK(ret == 0);
+       test_slab_list = test_slab_list_alloc(NULL, NULL, &def_slab_config, NULL, NULL, NULL, false, false);
        TEST_CHECK(test_slab_list != NULL);
        if (!test_slab_list) return;
 
@@ -286,14 +278,12 @@ static void test_reserve_mru(void)
 {
        test_slab_list_t        *test_slab_list;
        test_element_t          *test_elements[2];
-       int                     ret = -1;
 
        /*
         *      First use a slab list with reserve_mru = false to verify that the two reservations
         *      result in different elements being returned
         */
-       ret = test_slab_list_alloc(NULL, &test_slab_list, NULL, &def_slab_config, NULL, NULL, NULL, true, false);
-       TEST_CHECK(ret == 0);
+       test_slab_list = test_slab_list_alloc(NULL, NULL, &def_slab_config, NULL, NULL, NULL, true, false);
        TEST_CHECK(test_slab_list != NULL);
        if (!test_slab_list) return;
 
@@ -311,8 +301,7 @@ static void test_reserve_mru(void)
        /*
         *      Now use a slab list with reserve_mru = true
         */
-       ret = test_slab_list_alloc(NULL, &test_slab_list, NULL, &def_slab_config, NULL, NULL, NULL, true, true);
-       TEST_CHECK(ret == 0);
+       test_slab_list = test_slab_list_alloc(NULL, NULL, &def_slab_config, NULL, NULL, NULL, true, true);
        TEST_CHECK(test_slab_list != NULL);
        if (!test_slab_list) return;
 
@@ -336,10 +325,8 @@ static void test_free(void)
        test_slab_list_t        *test_slab_list;
        test_element_t          *test_element;
        test_uctx_t             test_uctx;
-       int                     ret = -1;
 
-       ret = test_slab_list_alloc(NULL, &test_slab_list, NULL, &def_slab_config, NULL, NULL, NULL, true, false);
-       TEST_CHECK(ret == 0);
+       test_slab_list = test_slab_list_alloc(NULL, NULL, &def_slab_config, NULL, NULL, NULL, true, false);
        TEST_CHECK(test_slab_list != NULL);
        if (!test_slab_list) return;
 
@@ -372,12 +359,10 @@ static void test_init(void)
        test_slab_list_t        *test_slab_list;
        test_element_t          *test_elements[2];
        test_conf_t             test_conf = { .initial = 10 };
-       int                     ret = -1;
        fr_slab_config_t        slab_config = def_slab_config;
 
        slab_config.elements_per_slab = 1;
-       ret = test_slab_list_alloc(NULL, &test_slab_list, NULL, &slab_config, test_element_alloc, NULL, &test_conf, false, false);
-       TEST_CHECK(ret == 0);
+       test_slab_list = test_slab_list_alloc(NULL, NULL, &slab_config, test_element_alloc, NULL, &test_conf, false, false);
        TEST_CHECK(test_slab_list != NULL);
        if (!test_slab_list) return;
 
@@ -412,12 +397,10 @@ static void test_reserve(void)
        test_slab_list_t        *test_slab_list;
        test_element_t          *test_elements[2];
        test_conf_t             test_conf = { .initial = 10 };
-       int                     ret = -1;
        fr_slab_config_t        slab_config = def_slab_config;
 
        slab_config.elements_per_slab = 1;
-       ret = test_slab_list_alloc(NULL, &test_slab_list, NULL, &slab_config, NULL, test_element_alloc, &test_conf, false, false);
-       TEST_CHECK(ret == 0);
+       test_slab_list = test_slab_list_alloc(NULL, NULL, &slab_config, NULL, test_element_alloc, &test_conf, false, false);
        TEST_CHECK(test_slab_list != NULL);
        if (!test_slab_list) return;
 
@@ -459,12 +442,10 @@ static void test_init_reserve(void)
        test_slab_list_t        *test_slab_list;
        test_element_t          *test_elements[2];
        test_conf_t             test_conf = { .initial = 10 };
-       int                     ret = -1;
        fr_slab_config_t        slab_config = def_slab_config;
 
        slab_config.elements_per_slab = 1;
-       ret = test_slab_list_alloc(NULL, &test_slab_list, NULL, &slab_config, test_element_alloc, test_element_reserve, &test_conf, false, false);
-       TEST_CHECK(ret == 0);
+       test_slab_list = test_slab_list_alloc(NULL, NULL, &slab_config, test_element_alloc, test_element_reserve, &test_conf, false, false);
        TEST_CHECK(test_slab_list != NULL);
        if (!test_slab_list) return;
 
@@ -500,15 +481,14 @@ static void test_clearup_1(void)
        fr_event_list_t         *el;
        test_slab_list_t        *test_slab_list;
        test_element_t          *test_elements[6];
-       int                     i, events, ret = -1;
+       int                     i, events;
        fr_slab_config_t        slab_config = def_slab_config;
 
        el = fr_event_list_alloc(ctx, NULL, NULL);
        fr_event_list_set_time_func(el, test_time);
 
        slab_config.max_elements = 6;
-       ret = test_slab_list_alloc(NULL, &test_slab_list, el, &slab_config, NULL, NULL, NULL, true, false);
-       TEST_CHECK(ret == 0);
+       test_slab_list = test_slab_list_alloc(NULL, el, &slab_config, NULL, NULL, NULL, true, false);
        TEST_CHECK(test_slab_list != NULL);
        if (!test_slab_list) return;
 
@@ -555,7 +535,7 @@ static void test_clearup_2(void)
        fr_event_list_t         *el;
        test_slab_list_t        *test_slab_list;
        test_element_t          *test_elements[20];
-       int                     i, events, ret = -1;
+       int                     i, events;
        fr_slab_config_t        slab_config = def_slab_config;
 
        el = fr_event_list_alloc(ctx, NULL, NULL);
@@ -563,8 +543,7 @@ static void test_clearup_2(void)
 
        slab_config.min_elements = 16;
        slab_config.max_elements = 20;
-       ret = test_slab_list_alloc(NULL, &test_slab_list, el, &slab_config, NULL, NULL, NULL, true, false);
-       TEST_CHECK(ret == 0);
+       test_slab_list = test_slab_list_alloc(NULL, el, &slab_config, NULL, NULL, NULL, true, false);
        TEST_CHECK(test_slab_list != NULL);
        if (!test_slab_list) return;
 
@@ -621,7 +600,7 @@ static void test_clearup_3(void)
        fr_event_list_t         *el;
        test_slab_list_t        *test_slab_list;
        test_element_t          *test_elements[20];
-       int                     i, events, ret = -1;
+       int                     i, events;
        fr_slab_config_t        slab_config = def_slab_config;
 
        el = fr_event_list_alloc(ctx, NULL, NULL);
@@ -629,8 +608,7 @@ static void test_clearup_3(void)
 
        slab_config.min_elements = 0;
        slab_config.max_elements = 20;
-       ret = test_slab_list_alloc(NULL, &test_slab_list, el, &slab_config, NULL, NULL, NULL, true, false);
-       TEST_CHECK(ret == 0);
+       test_slab_list = test_slab_list_alloc(NULL, el, &slab_config, NULL, NULL, NULL, true, false);
        TEST_CHECK(test_slab_list != NULL);
        if (!test_slab_list) return;
 
@@ -716,7 +694,7 @@ static void test_realloc(void)
        fr_event_list_t         *el;
        test_slab_list_t        *test_slab_list;
        test_element_t          *test_elements[20];
-       int                     i, events, ret = -1;
+       int                     i, events;
        fr_slab_config_t        slab_config = def_slab_config;
 
        el = fr_event_list_alloc(ctx, NULL, NULL);
@@ -724,8 +702,7 @@ static void test_realloc(void)
 
        slab_config.min_elements = 0;
        slab_config.max_elements = 20;
-       ret = test_slab_list_alloc(NULL, &test_slab_list, el, &slab_config, NULL, NULL, NULL, true, false);
-       TEST_CHECK(ret == 0);
+       test_slab_list = test_slab_list_alloc(NULL, el, &slab_config, NULL, NULL, NULL, true, false);
        TEST_CHECK(test_slab_list != NULL);
        if (!test_slab_list) return;
 
@@ -777,14 +754,12 @@ static void test_child_alloc(void)
 {
        test_slab_list_t        *test_slab_list;
        test_element_t          *test_elements[2];
-       int                     ret = -1;
        fr_slab_config_t        slab_config = def_slab_config;
 
        slab_config.max_elements = 2;
        slab_config.num_children = 1;
        slab_config.child_pool_size = 128;
-       ret = test_slab_list_alloc(NULL, &test_slab_list, NULL, &slab_config, NULL, NULL, NULL, false, false);
-       TEST_CHECK(ret == 0);
+       test_slab_list = test_slab_list_alloc(NULL, NULL, &slab_config, NULL, NULL, NULL, false, false);
        TEST_CHECK(test_slab_list != NULL);
        if (!test_slab_list) return;
 

diff --git a/src/modules/rlm_imap/rlm_imap.c b/src/modules/rlm_imap/rlm_imap.c
index 4534e527f2b54e1c505bda20ca71ee21d9ef9d5c..93b4c4a4201874970560df5df448b392c693d522 100644 (file)
--- a/src/modules/rlm_imap/rlm_imap.c
+++ b/src/modules/rlm_imap/rlm_imap.c
@@ -247,8 +247,9 @@ static int mod_thread_instantiate(module_thread_inst_ctx_t const *mctx)
        rlm_imap_thread_t       *t = talloc_get_type_abort(mctx->thread, rlm_imap_thread_t);
        fr_curl_handle_t        *mhandle;
 
-       if (imap_slab_list_alloc(t, &t->slab, mctx->el, &inst->conn_config.reuse,
-                                   imap_conn_alloc, NULL, inst, false, false) < 0) {
+       if (!(t->slab = imap_slab_list_alloc(t, mctx->el, &inst->conn_config.reuse,
+                                            imap_conn_alloc, NULL, inst,
+                                            false, false))) {
                ERROR("Connection handle pool instantiation failed");
                return -1;
        }

diff --git a/src/modules/rlm_rest/rlm_rest.c b/src/modules/rlm_rest/rlm_rest.c
index b3bea1a213351cab7a3b6826c67b5c25dbce59ae..360fec27355287d037ea6feb2796ef2cdc53e02e 100644 (file)
--- a/src/modules/rlm_rest/rlm_rest.c
+++ b/src/modules/rlm_rest/rlm_rest.c
@@ -1165,8 +1165,8 @@ static int mod_thread_instantiate(module_thread_inst_ctx_t const *mctx)
 
        t->inst = inst;
 
-       if (rest_slab_list_alloc(t, &t->slab, mctx->el, &inst->conn_config.reuse,
-                                rest_conn_alloc, NULL, inst, false, false) < 0) {
+       if (!(t->slab = rest_slab_list_alloc(t, mctx->el, &inst->conn_config.reuse,
+                                            rest_conn_alloc, NULL, inst, false, false))) {
                ERROR("Connection handle pool instantiation failed");
                return -1;
        }

diff --git a/src/modules/rlm_smtp/rlm_smtp.c b/src/modules/rlm_smtp/rlm_smtp.c
index f7369ce5e6f48159ac8fd98b94a730954c5237b9..3786d67b425b297e91a357ab0d7c75ff698decef 100644 (file)
--- a/src/modules/rlm_smtp/rlm_smtp.c
+++ b/src/modules/rlm_smtp/rlm_smtp.c
@@ -1205,13 +1205,15 @@ static int mod_thread_instantiate(module_thread_inst_ctx_t const *mctx)
        rlm_smtp_thread_t       *t = talloc_get_type_abort(mctx->thread, rlm_smtp_thread_t);
        fr_curl_handle_t        *mhandle;
 
-       if (smtp_slab_list_alloc(t, &t->slab_onetime, mctx->el, &inst->conn_config.reuse,
-                                   smtp_onetime_conn_alloc, smtp_onetime_conn_init, inst, false, false) < 0) {
+       if (!(t->slab_onetime = smtp_slab_list_alloc(t, mctx->el, &inst->conn_config.reuse,
+                                                    smtp_onetime_conn_alloc, smtp_onetime_conn_init,
+                                                    inst, false, false))) {
                ERROR("Connection handle pool instantiation failed");
                return -1;
        }
-       if (smtp_slab_list_alloc(t, &t->slab_persist, mctx->el, &inst->conn_config.reuse,
-                                   smtp_persist_conn_alloc, smtp_persist_conn_init, inst, false, true) < 0) {
+       if (!(t->slab_persist = smtp_slab_list_alloc(t, mctx->el, &inst->conn_config.reuse,
+                                                    smtp_persist_conn_alloc, smtp_persist_conn_init,
+                                                    inst, false, true))) {
                ERROR("Connection handle pool instantiation failed");
                return -1;
        }