Add more verification calls for value box lists

diff --git a/src/lib/unlang/map.c b/src/lib/unlang/map.c
index b1199745617ac6163ed98089237084f230f2a424..6c3ff9ff64567ba97d4345a18b069cf978ddd825 100644 (file)
--- a/src/lib/unlang/map.c
+++ b/src/lib/unlang/map.c
@@ -297,13 +297,9 @@ static unlang_action_t map_proc_apply(rlm_rcode_t *p_result, request_t *request,
        /*
         *      FIXME - We don't yet support async LHS/RHS expansions for map procs
         */
-#ifndef NDEBUG
-       if (!fr_dlist_empty(&map_proc_state->src_result)) fr_dlist_verify(&map_proc_state->src_result);
-#endif
+       FR_DLIST_VERIFY(&map_proc_state->src_result);
        *p_result = map_proc(request, gext->proc_inst, &map_proc_state->src_result);
-#ifndef NDEBUG
-       if (!fr_dlist_empty(&map_proc_state->src_result)) fr_dlist_verify(&map_proc_state->src_result);
-#endif
+       FR_DLIST_VERIFY(&map_proc_state->src_result);
 
        return *p_result == RLM_MODULE_YIELD ? UNLANG_ACTION_YIELD : UNLANG_ACTION_CALCULATE_RESULT;
 }

diff --git a/src/lib/unlang/xlat_eval.c b/src/lib/unlang/xlat_eval.c
index 05ec78fd5172032a88696cafadb994df6e3b3660..41f569ca6cf2a3be655614ff1d2fe4d6ea10110c 100644 (file)
--- a/src/lib/unlang/xlat_eval.c
+++ b/src/lib/unlang/xlat_eval.c
@@ -923,9 +923,9 @@ xlat_action_t xlat_frame_eval_resume(TALLOC_CTX *ctx, fr_dcursor_t *out,
         *      to debug problems if they free or change elements
         *      and don't remove them from the list.
         */
-       fr_dlist_verify(result);
+       VALUE_BOX_LIST_VERIFY(result);
        xa = resume(ctx, out, request, exp->call.inst, thread_inst->data, result, rctx);
-       fr_dlist_verify(result);
+       VALUE_BOX_LIST_VERIFY(result);
 
        RDEBUG2("EXPAND %%{%s:...}", exp->call.func->name);
        switch (xa) {
@@ -984,7 +984,7 @@ xlat_action_t xlat_frame_eval_repeat(TALLOC_CTX *ctx, fr_dcursor_t *out,
                        ssize_t         slen;
 
                        if (!fr_dlist_empty(result)) {
-                               fr_dlist_verify(result);
+                               VALUE_BOX_LIST_VERIFY(result);
                                result_str = fr_value_box_list_aprint(NULL, result, NULL, NULL);
                                if (!result_str) return XLAT_ACTION_FAIL;
                        } else {
@@ -1024,6 +1024,7 @@ xlat_action_t xlat_frame_eval_repeat(TALLOC_CTX *ctx, fr_dcursor_t *out,
                                fr_value_box_bstrdup_buffer_shallow(NULL, value, NULL, str, false);
                        }
 
+                       VALUE_BOX_VERIFY(value);
                        fr_dcursor_append(out, value);                  /* Append the result of the expansion */
                        talloc_free(result_str);
                        xlat_debug_log_result(request, value);
@@ -1049,8 +1050,7 @@ xlat_action_t xlat_frame_eval_repeat(TALLOC_CTX *ctx, fr_dcursor_t *out,
                         */
                        if (RDEBUG_ENABLED2) fr_value_box_list_acopy(NULL, &result_copy, result);
 
-                       fr_dlist_verify(result);
-
+                       VALUE_BOX_LIST_VERIFY(result);
                        xa = xlat_process_args(ctx, result, request, node->call.func->input_type, node->call.func->args);
                        if (xa == XLAT_ACTION_FAIL) {
                                if (RDEBUG_ENABLED2) fr_dlist_talloc_free(&result_copy);
@@ -1059,7 +1059,7 @@ xlat_action_t xlat_frame_eval_repeat(TALLOC_CTX *ctx, fr_dcursor_t *out,
 
                        xa = node->call.func->func.async(ctx, out, request,
                                                         node->call.inst->data, thread_inst->data, result);
-                       fr_dlist_verify(result);
+                       VALUE_BOX_LIST_VERIFY(result);
 
                        if (RDEBUG_ENABLED2) {
                                xlat_debug_log_expansion(request, *in, &result_copy);
@@ -1123,7 +1123,7 @@ xlat_action_t xlat_frame_eval_repeat(TALLOC_CTX *ctx, fr_dcursor_t *out,
                xlat_debug_log_expansion(request, *in, NULL);
                xlat_debug_log_list_result(request, result);
 
-               fr_dlist_verify(result);
+               VALUE_BOX_LIST_VERIFY(result);
                fr_dcursor_init(&from, result);
                fr_dcursor_merge(out, &from);
                fr_assert(fr_dlist_empty(result));
@@ -1138,7 +1138,8 @@ xlat_action_t xlat_frame_eval_repeat(TALLOC_CTX *ctx, fr_dcursor_t *out,
                           node->fmt);
 
                fr_assert(!fr_dlist_empty(result));
-               fr_dlist_verify(result);
+
+               VALUE_BOX_LIST_VERIFY(result);
 
                /*
                 *      If the input is a series of xlat expansions,
@@ -1152,7 +1153,6 @@ xlat_action_t xlat_frame_eval_repeat(TALLOC_CTX *ctx, fr_dcursor_t *out,
                        MEM(value = fr_value_box_alloc_null(ctx));
 
                        if (!fr_dlist_empty(result)) {
-                               fr_dlist_verify(result);
                                str = fr_value_box_list_aprint(value, result, NULL, NULL);
                                if (!str) return XLAT_ACTION_FAIL;
                        } else {
@@ -1163,6 +1163,7 @@ xlat_action_t xlat_frame_eval_repeat(TALLOC_CTX *ctx, fr_dcursor_t *out,
                        fr_dlist_talloc_free(result);
                        fr_dlist_insert_head(result, value);
                }
+               VALUE_BOX_LIST_VERIFY(result);
 
                MEM(value = fr_value_box_alloc(ctx, FR_TYPE_GROUP, NULL, false));
                fr_dlist_move(&value->vb_group, result);

diff --git a/src/lib/util/dlist.h b/src/lib/util/dlist.h
index 966b595dd055cfac410b85dd3b12291cf06ab08a..bb1889e4e7137405f4db6b0cce28db0bbb813060 100644 (file)
--- a/src/lib/util/dlist.h
+++ b/src/lib/util/dlist.h
@@ -565,22 +565,26 @@ static inline void *fr_dlist_replace(fr_dlist_head_t *list_head, void *item, voi
  * Does nothing if the list was not initialised with #fr_dlist_talloc_init.
  */
 #ifndef TALLOC_GET_TYPE_ABORT_NOOP
-static inline CC_HINT(nonnull) void fr_dlist_verify(fr_dlist_head_t *list_head)
+static inline CC_HINT(nonnull) void fr_dlist_verify(char const *file, int line, fr_dlist_head_t *list_head)
 {
        void *item;
 
        if (!list_head->type) return;
 
+       fr_assert_msg(fr_dlist_initialised(list_head), "CONSISTENCY CHECK FAILED %s[%i]: dlist not initialised",
+                     file, line);
+
        for (item = fr_dlist_head(list_head);
             item;
             item = fr_dlist_next(list_head, item)) {
             item = _talloc_get_type_abort(item, list_head->type, __location__);
        }
 }
+#  define FR_DLIST_VERIFY(_head) fr_dlist_verify(__FILE__, __LINE__, _head)
 #elif !defined(NDEBUG)
-#  define fr_dlist_verify(_head) fr_assert(_head)
+#  define FR_DLIST_VERIFY(_head) fr_assert(_head)
 #else
-#  define fr_dlist_verify(_head)
+#  define FR_DLIST_VERIFY(_head)
 #endif
 
 /** Merge two lists, inserting the tail of one into the other

diff --git a/src/lib/util/value.c b/src/lib/util/value.c
index 5488a2888bb950cb76a9417dca11625b3566a9be..6a0a325ac60044ec1bd677be23867152638ec8c3 100644 (file)
--- a/src/lib/util/value.c
+++ b/src/lib/util/value.c
@@ -4804,8 +4804,6 @@ ssize_t fr_value_box_print(fr_sbuff_t *out, fr_value_box_t const *data, fr_sbuff
                fr_value_box_t  vb;
                ssize_t         slen;
 
-               memset(&vb, 0, sizeof(vb));
-
                /*
                 *      Be lazy by just converting it to a string, and then printing the string.
                 */
@@ -5176,3 +5174,48 @@ bool fr_value_box_list_tainted(fr_value_box_list_t const *head)
 
        return false;
 }
+
+/** Validation function to check that a fr_value_box_t is correctly initialised
+ *
+ * @note Do not add talloc checks here.  fr_value_box_t may be allocated on the stack.
+ */
+void value_box_verify(char const *file, int line, fr_value_box_t const *vb)
+{
+       fr_fatal_assert_msg(vb, "CONSISTENCY CHECK FAILED %s[%i]: fr_value_box_t pointer was NULL", file, line);
+
+       switch (vb->type) {
+       case FR_TYPE_STRING:
+               fr_fatal_assert_msg(vb->vb_strvalue, "CONSISTENCY CHECK FAILED %s[%i]: fr_value_box_t strvalue field "
+                                   "was NULL", file, line);
+               fr_fatal_assert_msg(vb->vb_strvalue[vb->vb_length] == '\0',
+                                   "CONSISTENCY CHECK FAILED %s[%i]: fr_value_box_t strvalue field "
+                                   "not null terminated", file, line);
+               break;
+
+       case FR_TYPE_OCTETS:
+               fr_fatal_assert_msg(vb->vb_octets, "CONSISTENCY CHECK FAILED %s[%i]: fr_value_box_t octets field "
+                                   "was NULL", file, line);
+               break;
+
+       case FR_TYPE_VOID:
+               fr_fatal_assert_msg(vb->vb_void, "CONSISTENCY CHECK FAILED %s[%i]: fr_value_box_t ptr field "
+                                   "was NULL", file, line);
+               break;
+
+       case FR_TYPE_GROUP:
+               value_box_list_verify(file, line, &vb->vb_group);
+               break;
+
+       default:
+               break;
+       }
+}
+
+void value_box_list_verify(char const *file, int line, fr_value_box_list_t const *list)
+{
+       fr_value_box_t const *vb;
+
+       FR_DLIST_VERIFY(list);
+
+       while ((vb = fr_dlist_next(list, vb))) value_box_verify(file, line, list);
+}

diff --git a/src/lib/util/value.h b/src/lib/util/value.h
index dbb25500c23d14a59925bd89901d47b22a1fdcbd..466fce1d4b86717636a30f9e418f1ac12edfa6d8 100644 (file)
--- a/src/lib/util/value.h
+++ b/src/lib/util/value.h
@@ -165,6 +165,7 @@ struct value_box_s {
  */
 #define vb_strvalue                            datum.strvalue
 #define vb_octets                              datum.octets
+#define vb_void                                        datum.ptr
 #define vb_group                               datum.children
 
 #define vb_ip                                  datum.ip
@@ -735,8 +736,9 @@ bool                fr_value_box_list_tainted(fr_value_box_list_t const *head);
 int            fr_value_box_list_flatten_argv(TALLOC_CTX *ctx, char ***argv_p, fr_value_box_list_t const *in);
 /** @} */
 
-/*
- *     Printing
+/** @name Print the value of a value box as a string
+ *
+ * @{
  */
 ssize_t                fr_value_box_print(fr_sbuff_t *out, fr_value_box_t const *data, fr_sbuff_escape_rules_t const *e_rules) CC_HINT(nonnull(1,2));
 
@@ -753,6 +755,7 @@ static inline size_t fr_value_box_aprint_quoted(TALLOC_CTX *ctx, char **out,
 {
        SBUFF_OUT_TALLOC_FUNC_NO_LEN_DEF(fr_value_box_print_quoted, data, quote)
 }
+/** @} */
 /** @name Hashing
  *
  * @{
@@ -760,6 +763,22 @@ static inline size_t fr_value_box_aprint_quoted(TALLOC_CTX *ctx, char **out,
 uint32_t       fr_value_box_hash_update(fr_value_box_t const *vb, uint32_t hash);
 /** @} */
 
+void value_box_verify(char const *file, int line, fr_value_box_t const *vb);
+void value_box_list_verify(char const *file, int line, fr_value_box_list_t const *list);
+
+#ifdef WITH_VERIFY_PTR
+#  define VALUE_BOX_VERIFY(_x) value_box_verify(__FILE__, __LINE__, _x)
+#  define VALUE_BOX_LIST_VERIFY(_x) value_box_list_verify(__FILE__, __LINE__, _x)
+#else
+/*
+ *  Even if were building without WITH_VERIFY_PTR
+ *  the pointer must not be NULL when these various macros are used
+ *  so we can add some sneaky asserts.
+ */
+#  define VALUE_BOX_VERIFY(_x) fr_assert(_x)
+#  define VALUE_BOX_LIST_VERIFY(_x) fr_assert(_x)
+#endif
+
 #undef _CONST
 
 #ifdef __cplusplus