CLEANUP: checks: fix double usage of cur / current_step in tcp-checks
This cleanup is a preliminary requirement to the upcoming fixes for
the bug that affect tcp-check's improper use of lists. It will have
to be backported to 1.5 though it will not easily apply.
There are two variables pointing to the current rule within the loop,
and either one or the other is used depending on the code blocks,
making it much harder to apply checks to fix the list walking bug.
So first get rid of "cur" and only focus on current_step.
(cherry picked from commit ce8c42a37a44a1e0cb94e81abb7cc2baf3d0ef80)
[wt: 1.5 doesn't have comments so this patch differs significantly
from 1.6, but it's needed for the next batch of fixes]
diff --git a/src/checks.c b/src/checks.c
index 8b53f97..cfdfe8c 100644
--- a/src/checks.c
+++ b/src/checks.c
@@ -1859,7 +1859,7 @@
static void tcpcheck_main(struct connection *conn)
{
char *contentptr;
- struct tcpcheck_rule *cur, *next;
+ struct tcpcheck_rule *next;
int done = 0, ret = 0;
struct check *check = conn->owner;
struct server *s = check->server;
@@ -1916,15 +1916,11 @@
check->bo->o = 0;
check->bi->p = check->bi->data;
check->bi->i = 0;
- cur = check->current_step = LIST_ELEM(head->n, struct tcpcheck_rule *, list);
+ check->current_step = LIST_ELEM(head->n, struct tcpcheck_rule *, list);
t->expire = tick_add(now_ms, MS_TO_TICKS(check->inter));
if (s->proxy->timeout.check)
t->expire = tick_add_ifset(now_ms, s->proxy->timeout.check);
}
- /* keep on processing step */
- else {
- cur = check->current_step;
- }
/* It's only the rules which will enable send/recv */
__conn_data_stop_both(conn);
@@ -1934,7 +1930,7 @@
* or if we're about to send a string that does not fit in the remaining space.
*/
if (check->bo->o &&
- (&cur->list == head ||
+ (&check->current_step->list == head ||
check->current_step->action != TCPCHK_ACT_SEND ||
check->current_step->string_len >= buffer_total_space(check->bo))) {
@@ -1949,14 +1945,17 @@
}
/* did we reach the end ? If so, let's check that everything was sent */
- if (&cur->list == head) {
+ if (&check->current_step->list == head) {
if (check->bo->o)
goto out_need_io;
break;
}
+ /* have 'next' point to the next rule or NULL if we're on the
+ * last one, connect() needs this.
+ */
+ next = (struct tcpcheck_rule *)check->current_step->list.n;
+
- /* have 'next' point to the next rule or NULL if we're on the last one */
- next = (struct tcpcheck_rule *)cur->list.n;
if (&next->list == head)
next = NULL;
@@ -2058,8 +2057,7 @@
}
/* allow next rule */
- cur = (struct tcpcheck_rule *)cur->list.n;
- check->current_step = cur;
+ check->current_step = (struct tcpcheck_rule *)check->current_step->list.n;
/* don't do anything until the connection is established */
if (!(conn->flags & CO_FL_CONNECTED)) {
@@ -2113,8 +2111,7 @@
*check->bo->p = '\0'; /* to make gdb output easier to read */
/* go to next rule and try to send */
- cur = (struct tcpcheck_rule *)cur->list.n;
- check->current_step = cur;
+ check->current_step = (struct tcpcheck_rule *)check->current_step->list.n;
} /* end 'send' */
else if (check->current_step->action == TCPCHK_ACT_EXPECT) {
if (unlikely(check->result == CHK_RES_FAILED))
@@ -2167,14 +2164,14 @@
goto out_end_tcpcheck;
}
- if (!done && (cur->string != NULL) && (check->bi->i < cur->string_len) )
+ if (!done && (check->current_step->string != NULL) && (check->bi->i < check->current_step->string_len) )
continue; /* try to read more */
tcpcheck_expect:
- if (cur->string != NULL)
- ret = my_memmem(contentptr, check->bi->i, cur->string, cur->string_len) != NULL;
- else if (cur->expect_regex != NULL)
- ret = regex_exec(cur->expect_regex, contentptr);
+ if (check->current_step->string != NULL)
+ ret = my_memmem(contentptr, check->bi->i, check->current_step->string, check->current_step->string_len) != NULL;
+ else if (check->current_step->expect_regex != NULL)
+ ret = regex_exec(check->current_step->expect_regex, contentptr);
if (!ret && !done)
continue; /* try to read more */
@@ -2182,11 +2179,11 @@
/* matched */
if (ret) {
/* matched but we did not want to => ERROR */
- if (cur->inverse) {
+ if (check->current_step->inverse) {
/* we were looking for a string */
- if (cur->string != NULL) {
+ if (check->current_step->string != NULL) {
chunk_printf(&trash, "TCPCHK matched unwanted content '%s' at step %d",
- cur->string, tcpcheck_get_step_id(s));
+ check->current_step->string, tcpcheck_get_step_id(s));
}
else {
/* we were looking for a regex */
@@ -2198,8 +2195,9 @@
}
/* matched and was supposed to => OK, next step */
else {
- cur = (struct tcpcheck_rule*)cur->list.n;
- check->current_step = cur;
+ /* allow next rule */
+ check->current_step = (struct tcpcheck_rule *)check->current_step->list.n;
+
if (check->current_step->action == TCPCHK_ACT_EXPECT)
goto tcpcheck_expect;
__conn_data_stop_recv(conn);
@@ -2208,9 +2206,10 @@
else {
/* not matched */
/* not matched and was not supposed to => OK, next step */
- if (cur->inverse) {
- cur = (struct tcpcheck_rule*)cur->list.n;
- check->current_step = cur;
+ if (check->current_step->inverse) {
+ /* allow next rule */
+ check->current_step = (struct tcpcheck_rule *)check->current_step->list.n;
+
if (check->current_step->action == TCPCHK_ACT_EXPECT)
goto tcpcheck_expect;
__conn_data_stop_recv(conn);
@@ -2218,9 +2217,9 @@
/* not matched but was supposed to => ERROR */
else {
/* we were looking for a string */
- if (cur->string != NULL) {
+ if (check->current_step->string != NULL) {
chunk_printf(&trash, "TCPCHK did not match content '%s' at step %d",
- cur->string, tcpcheck_get_step_id(s));
+ check->current_step->string, tcpcheck_get_step_id(s));
}
else {
/* we were looking for a regex */