|
 |
61475b9 |
commit c0f670c233e9562118648af641d4f8d182350f31
|
|
|
61475b9 |
Author: Arran Cudbard-Bell <a.cudbardb@freeradius.org>
|
|
|
61475b9 |
Date: Fri May 16 11:38:22 2014 +0100
|
|
|
61475b9 |
|
|
|
61475b9 |
Make the foreach code slightly more sane. Reliably reproduces the issue described by #639
|
|
|
61475b9 |
|
|
|
61475b9 |
diff --git a/src/main/modcall.c b/src/main/modcall.c
|
|
|
61475b9 |
index 3dd0828..7a44bf2 100644
|
|
|
61475b9 |
--- a/src/main/modcall.c
|
|
|
61475b9 |
+++ b/src/main/modcall.c
|
|
|
61475b9 |
@@ -608,9 +608,9 @@ redo:
|
|
|
61475b9 |
*/
|
|
|
61475b9 |
if (c->type == MOD_FOREACH) {
|
|
|
61475b9 |
int i, foreach_depth = -1;
|
|
|
61475b9 |
- VALUE_PAIR *vps, **tail, *vp;
|
|
|
61475b9 |
+ VALUE_PAIR *vps, *vp;
|
|
|
61475b9 |
modcall_stack_entry_t *next = NULL;
|
|
|
61475b9 |
- vp_cursor_t cursor;
|
|
|
61475b9 |
+ vp_cursor_t cursor, copy;
|
|
|
61475b9 |
modgroup *g = mod_callabletogroup(c);
|
|
|
61475b9 |
|
|
|
61475b9 |
if (depth >= MODCALL_STACK_MAX) {
|
|
|
61475b9 |
@@ -645,31 +645,35 @@ redo:
|
|
|
61475b9 |
}
|
|
|
61475b9 |
|
|
|
61475b9 |
/*
|
|
|
61475b9 |
- * Copy the VPs from the original request.
|
|
|
61475b9 |
+ * Copy the VPs from the original request, this ensures deterministic
|
|
|
61475b9 |
+ * behaviour if someone decides to add or remove VPs in the set were
|
|
|
61475b9 |
+ * iterating over.
|
|
|
61475b9 |
*/
|
|
|
61475b9 |
- fr_cursor_init(&cursor, &vp);
|
|
|
61475b9 |
- /* Prime the cursor. */
|
|
|
61475b9 |
- cursor.found = cursor.current;
|
|
|
61475b9 |
-
|
|
|
61475b9 |
vps = NULL;
|
|
|
61475b9 |
- tail = &vps;
|
|
|
61475b9 |
|
|
|
61475b9 |
- while (vp) {
|
|
|
61475b9 |
- *tail = paircopyvp(request, vp);
|
|
|
61475b9 |
- if (!*tail) break;
|
|
|
61475b9 |
+ fr_cursor_init(&cursor, &vp);
|
|
|
61475b9 |
|
|
|
61475b9 |
- tail = &((*tail)->next); /* really should be using cursors... */
|
|
|
61475b9 |
+ /* Prime the cursor. */
|
|
|
61475b9 |
+ cursor.found = cursor.current;
|
|
|
61475b9 |
+ for (fr_cursor_init(©, &vps;;
|
|
|
61475b9 |
+ vp;
|
|
|
61475b9 |
+ vp = fr_cursor_next_by_da(&cursor, vp->da, g->vpt->attribute.tag)) {
|
|
|
61475b9 |
+ VALUE_PAIR *tmp;
|
|
|
61475b9 |
|
|
|
61475b9 |
- vp = fr_cursor_next_by_da(&cursor, vp->da, g->vpt->attribute.tag);
|
|
|
61475b9 |
+ MEM(tmp = paircopyvp(request, vp));
|
|
|
61475b9 |
+ fr_cursor_insert(©, tmp);
|
|
|
61475b9 |
}
|
|
|
61475b9 |
|
|
|
61475b9 |
- RDEBUG2("%.*sforeach %s ", depth + 1, modcall_spaces,
|
|
|
61475b9 |
- c->name);
|
|
|
61475b9 |
+ RDEBUG2("%.*sforeach %s ", depth + 1, modcall_spaces, c->name);
|
|
|
61475b9 |
+
|
|
|
61475b9 |
rad_assert(vps != NULL);
|
|
|
61475b9 |
|
|
|
61475b9 |
- for (vp = fr_cursor_init(&cursor, &vps;;
|
|
|
61475b9 |
+ /*
|
|
|
61475b9 |
+ * This is the actual body of the foreach loop
|
|
|
61475b9 |
+ */
|
|
|
61475b9 |
+ for (vp = fr_cursor_first(©);
|
|
|
61475b9 |
vp != NULL;
|
|
|
61475b9 |
- vp = fr_cursor_next(&cursor)) {
|
|
|
61475b9 |
+ vp = fr_cursor_next(©)) {
|
|
|
61475b9 |
#ifndef NDEBUG
|
|
|
61475b9 |
if (fr_debug_flag >= 2) {
|
|
|
61475b9 |
char buffer[1024];
|
|
|
61475b9 |
@@ -709,7 +713,7 @@ redo:
|
|
|
61475b9 |
}
|
|
|
61475b9 |
} /* loop over VPs */
|
|
|
61475b9 |
|
|
|
61475b9 |
- talloc_free(vps);
|
|
|
61475b9 |
+ pairfree(&vps;;
|
|
|
61475b9 |
|
|
|
61475b9 |
rad_assert(next != NULL);
|
|
|
61475b9 |
result = next->result;
|
|
|
61475b9 |
|
|
|
61475b9 |
commit 860f645668b9604c897c4ee1338ecfeb88cdd75a
|
|
|
61475b9 |
Author: Arran Cudbard-Bell <a.cudbardb@freeradius.org>
|
|
|
61475b9 |
Date: Fri May 16 12:32:12 2014 +0100
|
|
|
61475b9 |
|
|
|
61475b9 |
Don't free foreach VPs on break #639
|
|
|
61475b9 |
|
|
|
61475b9 |
Wwe go back up the stack in an orderly way and don't need this hack anymore
|
|
|
61475b9 |
|
|
|
61475b9 |
diff --git a/src/main/modcall.c b/src/main/modcall.c
|
|
|
61475b9 |
index 7a44bf2..5785643 100644
|
|
|
61475b9 |
--- a/src/main/modcall.c
|
|
|
61475b9 |
+++ b/src/main/modcall.c
|
|
|
61475b9 |
@@ -732,9 +732,7 @@ redo:
|
|
|
61475b9 |
for (i = 8; i >= 0; i--) {
|
|
|
61475b9 |
copy_p = request_data_get(request, radius_get_vp, i);
|
|
|
61475b9 |
if (copy_p) {
|
|
|
61475b9 |
- RDEBUG2("%.*s # break Foreach-Variable-%d", depth + 1,
|
|
|
61475b9 |
- modcall_spaces, i);
|
|
|
61475b9 |
- pairfree(copy_p);
|
|
|
61475b9 |
+ RDEBUG2("%.*s # break Foreach-Variable-%d", depth + 1, modcall_spaces, i);
|
|
|
61475b9 |
break;
|
|
|
61475b9 |
}
|
|
|
61475b9 |
}
|
|
|
61475b9 |
|