Add generic talloc_list_get_type_abort function

...and use it to verify the state of the result xlat eval result list

diff --git a/src/include/talloc.h b/src/include/talloc.h
index b075e33e92167739aa7b2569bc7d0ca4639f7d28..a324d41bf16b072b7f2bf410a21d978eb5397f24 100644 (file)
--- a/src/include/talloc.h
+++ b/src/include/talloc.h
@@ -54,7 +54,7 @@ void          talloc_const_free(void const *ptr);
  *
  * @param[in] _head    of list to free.  Will set memory it points to to be NULL.
  */
-#define                talloc_list_free(_head) _talloc_list_free((void **)_head, offsetof(__typeof__(**(_head)), next))
+#define        talloc_list_free(_head) _talloc_list_free((void **)_head, offsetof(__typeof__(**(_head)), next))
 
 static inline void _talloc_list_free(void **head, size_t offset)
 {
@@ -68,6 +68,31 @@ static inline void _talloc_list_free(void **head, size_t offset)
        *head = NULL;
 }
 
+/** Verify a list of talloced structures are the correct type and are still valid
+ *
+ * @param[in] _head    of list to check.
+ * @param[in] _type    of talloced chunk we expect.
+ */
+#ifndef TALLOC_GET_TYPE_ABORT_NOOP
+#  define talloc_list_get_type_abort(_head, _type) (_type *)_talloc_list_get_type_abort(_head, offsetof(__typeof__(*(_head)), next), #_type, __location__)
+static inline void *_talloc_list_get_type_abort(void *head, size_t offset, char const *type, char const *location)
+{
+       void *v = head, *n;
+
+       if (!v) _talloc_get_type_abort(v, type, location);      /* Behave like the normal talloc_get_type_abort function */
+
+       while (v) {
+               n = *((void **)(((uint8_t *)(v)) + offset));
+               _talloc_get_type_abort(v, type, location);
+               v = n;
+       }
+
+       return head;
+}
+#else
+#  define talloc_get_type_abort(_head, _type)
+#endif
+
 /*
  *     talloc portability issues.  'const' is not part of the talloc
  *     type, but it is part of the pointer type.  But only if

diff --git a/src/main/xlat_eval.c b/src/main/xlat_eval.c
index 585a6ce9b0d77aeed5d4390c0e954165f8b483b3..a5db8a1036ed580700fe6d17a9c6459b72a1dd3e 100644 (file)
--- a/src/main/xlat_eval.c
+++ b/src/main/xlat_eval.c
@@ -512,7 +512,15 @@ xlat_action_t xlat_frame_eval_resume(TALLOC_CTX *ctx, fr_cursor_t *out,
        xlat_thread_inst_t      *thread_inst = xlat_thread_instance_find(exp);
        xlat_action_t           xa;
 
+       /*
+        *      It's important that callbacks leave the result list
+        *      in a valid state, as it leads to all kinds of hard
+        *      to debug problems if they free or change elements
+        *      and don't remove them from the list.
+        */
+       if (*result) (void) talloc_list_get_type_abort(*result, fr_value_box_t);
        xa = resume(ctx, out, request, exp->inst, thread_inst->data, result, rctx);
+       if (*result) (void) talloc_list_get_type_abort(*result, fr_value_box_t);
        switch (xa) {
        default:
                break;
@@ -570,6 +578,7 @@ xlat_action_t xlat_frame_eval_repeat(TALLOC_CTX *ctx, fr_cursor_t *out,
                        ssize_t         slen;
 
                        if (*result) {
+                               (void) talloc_list_get_type_abort(*result, fr_value_box_t);
                                result_str = fr_value_box_list_asprint(NULL, *result, NULL, '\0');
                                if (!result_str) return XLAT_ACTION_FAIL;
                        } else {
@@ -591,8 +600,8 @@ xlat_action_t xlat_frame_eval_repeat(TALLOC_CTX *ctx, fr_cursor_t *out,
                                talloc_free(str);
                                return XLAT_ACTION_FAIL;
                        }
-                       if (slen == 0) break;   /* Zero length result */
-                       (void)talloc_get_type_abort(str, char);
+                       if (slen == 0) break;                           /* Zero length result */
+                       (void)talloc_get_type_abort(str, char);         /* Check output buffer is sane */
 
                        /*
                         *      Shrink the buffer
@@ -608,37 +617,32 @@ xlat_action_t xlat_frame_eval_repeat(TALLOC_CTX *ctx, fr_cursor_t *out,
 
                        RDEBUG2("EXPAND %%{%s:%pS}", node->fmt, result_str);
                        RDEBUG2("   --> %pV", value);
-                       fr_cursor_append(out, value);   /* Append the result of the expansion */
+                       fr_cursor_append(out, value);                   /* Append the result of the expansion */
                        talloc_free(result_str);
                }
                        break;
 
                case XLAT_FUNC_ASYNC:
                {
-                       xlat_action_t           action;
+                       xlat_action_t           xa;
                        xlat_thread_inst_t      *thread_inst;
 
                        RDEBUG2("EXPAND %%{%s:%pM}", node->xlat->name, *result);
 
                        thread_inst = xlat_thread_instance_find(node);
-                       action = node->xlat->func.async(ctx, out, request, node->inst, thread_inst->data, result);
-                       switch (action) {
+
+                       if (*result) (void) talloc_list_get_type_abort(*result, fr_value_box_t);
+                       xa = node->xlat->func.async(ctx, out, request, node->inst, thread_inst->data, result);
+                       if (*result) (void) talloc_list_get_type_abort(*result, fr_value_box_t);
+                       switch (xa) {
                        case XLAT_ACTION_FAIL:
                        case XLAT_ACTION_PUSH_CHILD:
                        case XLAT_ACTION_YIELD:
-                               return action;
+                               return xa;
 
-                       case XLAT_ACTION_DONE:          /* Process the result */
+                       case XLAT_ACTION_DONE:                          /* Process the result */
                                break;
                        }
-
-                       fr_cursor_next(out);            /* Wind to the start of this function's output */
-
-                       /*
-                        *      Print output if the function didn't yield
-                        *      else we need to print it in xlat_resume
-                        */
-
                        break;
                }
                }
@@ -671,6 +675,8 @@ xlat_action_t xlat_frame_eval_repeat(TALLOC_CTX *ctx, fr_cursor_t *out,
 
                        return XLAT_ACTION_PUSH_CHILD;
                }
+
+               if (*result) (void) talloc_list_get_type_abort(*result, fr_value_box_t);
                fr_cursor_init(&from, result);
                fr_cursor_merge(out, &from);
        }
@@ -796,7 +802,7 @@ xlat_action_t xlat_frame_eval(TALLOC_CTX *ctx, fr_cursor_t *out, xlat_exp_t cons
                {
                        fr_value_box_t *result = NULL;
 
-                       XLAT_DEBUG("** [%i] %s(func) - %%{%s: }", unlang_stack_depth(request), __FUNCTION__,
+                       XLAT_DEBUG("** [%i] %s(func) - %%{%s:...}", unlang_stack_depth(request), __FUNCTION__,
                                   node->fmt);
 
                        /*