Re: [PATCH] BUG/MINOR: build: Fix compilation with debug mode enabled

[Willy Tarreau]

Hi Cyril,

On Mon, Jul 23, 2018 at 10:04:34PM +0200, Cyril Bonté wrote:
> Some monthes ago, I began writing a compilation test script for haproxy, but
> as you may have noticed, I was not very available recently ;-)

Oh it happens to all of us unfortunately :-/

> I should be
> more available now. I'll try to finish this little work as it would have
> detected such type of error.

Great!

> The script parses the Makefile to find all USE_* settings and performs a
> compilation test for each one. I still have some work to do to prepare some
> compilations (dependencies like slz, deviceatlas, 51degrees, ...), but it
> looks to be already useful. I've now added DEBUG=-DDEBUG_FULL in the
> compilation options.
> 
> The main issue is that it takes hours on the tiny atom server I wanted to
> use for that job. But well, on my laptop it takes less that 2 minutes :
> that's acceptable, I've added it in the git hooks so it is executed each
> time I fetch commits from the repository.

Nice! A full build takes around 3-5 seconds on the build farm I have at
the office (I extended the initial distcc farm with the load generators).

> Some ideas for future versions :
> - randomly mix USE_* options: for example, it would have triggered an error
> to indicate an incompatbility between USE_DEVICEATLAS and USE_PCRE2.

I tend to think that randomly mixing settings will not detect much in fact.
If you have 20 settings, you have 1 million combinations. If a few of them
are incompatible, you'll very rarely meet them. However you may face some
which are expected to fail. Some very likely make sense and probably just
need to be hard-coded, especially once they have been reported to
occasionally fail. In the end you'll have less combinations with a higher
chance to detect failures by having just an iteration over all settings
one at a time and a selected set of combinations. At least that's how I
see it :-)

> - use different SSL libs/versions

Good point! I've done this a few times when we touched ssl_sock.c and
that's definitely needed.

Cheers,
Willy

Re: [PATCH] BUG/MINOR: build: Fix compilation with debug mode enabled

[Willy Tarreau]

On Fri, Jul 20, 2018 at 10:31:33AM +0200, Christopher Faulet wrote:
> Hi Willy,
> 
> Here is a patch to fix the compilation of HAProxy with the debug mode
> enabled. Some debug messages were still using the old buffers api.

It was left as an API conversion exercise for the reader :-)

Bah as you and Cyril regularly notice, I don't build with DEBUG_ALL
which is too verbose for me, and I regularly break it :-/

Sorry about this, patch merged, thanks!
Willy

[PATCH] BUG/MINOR: build: Fix compilation with debug mode enabled

[Christopher Faulet]

Hi Willy,

Here is a patch to fix the compilation of HAProxy with the debug mode 
enabled. Some debug messages were still using the old buffers api.


--
Christopher Faulet
>From deb61c7822acbfcc7fe0ff611eee51fd57773e72 Mon Sep 17 00:00:00 2001
From: Christopher Faulet 
Date: Fri, 20 Jul 2018 10:16:29 +0200
Subject: [PATCH] BUG/MINOR: build: Fix compilation with debug mode enabled

It remained some fragments of the old buffers API in debug messages, here and
there.
---
 src/backend.c|  4 ++--
 src/cli.c|  4 ++--
 src/flt_spoe.c   |  2 +-
 src/proto_http.c | 24 
 src/stream.c | 28 ++--
 src/tcp_rules.c  |  8 
 6 files changed, 35 insertions(+), 35 deletions(-)

diff --git a/src/backend.c b/src/backend.c
index e94c4c9b3..b2b7ba580 100644
--- a/src/backend.c
+++ b/src/backend.c
@@ -1399,13 +1399,13 @@ int tcp_persist_rdp_cookie(struct stream *s, struct channel *req, int an_bit)
 	uint32_t addr;
 	char *p;
 
-	DPRINTF(stderr,"[%u] %s: stream=%p b=%p, exp(r,w)=%u,%u bf=%08x bh=%d analysers=%02x\n",
+	DPRINTF(stderr,"[%u] %s: stream=%p b=%p, exp(r,w)=%u,%u bf=%08x bh=%lu analysers=%02x\n",
 		now_ms, __FUNCTION__,
 		s,
 		req,
 		req->rex, req->wex,
 		req->flags,
-		req->buf->i,
+		ci_data(req),
 		req->analysers);
 
 	if (s->flags & SF_ASSIGNED)
diff --git a/src/cli.c b/src/cli.c
index 0d3c95c22..d02842960 100644
--- a/src/cli.c
+++ b/src/cli.c
@@ -764,9 +764,9 @@ static void cli_io_handler(struct appctx *appctx)
 	}
 
  out:
-	DPRINTF(stderr, "%s@%d: st=%d, rqf=%x, rpf=%x, rqh=%d, rqs=%d, rh=%d, rs=%d\n",
+	DPRINTF(stderr, "%s@%d: st=%d, rqf=%x, rpf=%x, rqh=%lu, rqs=%lu, rh=%lu, rs=%lu\n",
 		__FUNCTION__, __LINE__,
-		si->state, req->flags, res->flags, req->buf->i, req->buf->o, res->buf->i, res->buf->o);
+		si->state, req->flags, res->flags, ci_data(req), co_data(req), ci_data(res), co_data(res));
 }
 
 /* This is called when the stream interface is closed. For instance, upon an
diff --git a/src/flt_spoe.c b/src/flt_spoe.c
index 63482f570..73e8f623a 100644
--- a/src/flt_spoe.c
+++ b/src/flt_spoe.c
@@ -2264,7 +2264,7 @@ spoe_encode_messages(struct stream *s, struct spoe_context *ctx,
 		agent->id, __FUNCTION__, s,
 		((ctx->flags & SPOE_CTX_FL_FRAGMENTED) ? "last fragment of" : "unfragmented"),
 		ctx->spoe_appctx, (agent->rt[tid].frame_size - FRAME_HDR_SIZE),
-		p - ctx->buffer->p);
+		p - b_head(>buffer));
 
 	b_set_data(>buffer, p - b_head(>buffer));
 	ctx->frag_ctx.curmsg = NULL;
diff --git a/src/proto_http.c b/src/proto_http.c
index 56035b9cb..6fa38dd23 100644
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -1613,13 +1613,13 @@ int http_wait_for_request(struct stream *s, struct channel *req, int an_bit)
 	struct http_msg *msg = >req;
 	struct hdr_ctx ctx;
 
-	DPRINTF(stderr,"[%u] %s: stream=%p b=%p, exp(r,w)=%u,%u bf=%08x bh=%d analysers=%02x\n",
+	DPRINTF(stderr,"[%u] %s: stream=%p b=%p, exp(r,w)=%u,%u bf=%08x bh=%lu analysers=%02x\n",
 		now_ms, __FUNCTION__,
 		s,
 		req,
 		req->rex, req->wex,
 		req->flags,
-		req->buf.i,
+		ci_data(req),
 		req->analysers);
 
 	/* we're speaking HTTP here, so let's speak HTTP to the client */
@@ -3477,13 +3477,13 @@ int http_process_req_common(struct stream *s, struct channel *req, int an_bit, s
 		goto return_prx_yield;
 	}
 
-	DPRINTF(stderr,"[%u] %s: stream=%p b=%p, exp(r,w)=%u,%u bf=%08x bh=%d analysers=%02x\n",
+	DPRINTF(stderr,"[%u] %s: stream=%p b=%p, exp(r,w)=%u,%u bf=%08x bh=%lu analysers=%02x\n",
 		now_ms, __FUNCTION__,
 		s,
 		req,
 		req->rex, req->wex,
 		req->flags,
-		req->buf.i,
+		ci_data(req),
 		req->analysers);
 
 	/* just in case we have some per-backend tracking */
@@ -3749,13 +3749,13 @@ int http_process_request(struct stream *s, struct channel *req, int an_bit)
 		return 0;
 	}
 
-	DPRINTF(stderr,"[%u] %s: stream=%p b=%p, exp(r,w)=%u,%u bf=%08x bh=%d analysers=%02x\n",
+	DPRINTF(stderr,"[%u] %s: stream=%p b=%p, exp(r,w)=%u,%u bf=%08x bh=%lu analysers=%02x\n",
 		now_ms, __FUNCTION__,
 		s,
 		req,
 		req->rex, req->wex,
 		req->flags,
-		req->buf.i,
+		ci_data(req),
 		req->analysers);
 
 	/*
@@ -4909,13 +4909,13 @@ int http_request_forward_body(struct stream *s, struct channel *req, int an_bit)
 	struct http_msg *msg = >txn->req;
 	int ret;
 
-	DPRINTF(stderr,"[%u] %s: stream=%p b=%p, exp(r,w)=%u,%u bf=%08x bh=%d analysers=%02x\n",
+	DPRINTF(stderr,"[%u] %s: stream=%p b=%p, exp(r,w)=%u,%u bf=%08x bh=%lu analysers=%02x\n",
 		now_ms, __FUNCTION__,
 		s,
 		req,
 		req->rex, req->wex,
 		req->flags,
-		req->buf.i,
+		ci_data(req),
 		req->analysers);
 
 	if (unlikely(msg->msg_state < HTTP_MSG_BODY))
@@ -5145,7 +5145,7 @@ int h