Collapse body_status and req_body_status into one type.

A lot of this is s/REQ_BODY_/BS_/g, followed by a reduction
pass to go from testing on specific status values to attributes
of the status value.

For instance the req.body caching code does not need to know if
how the transport sees this, chunked vs. eof for instance, it just
needs to know that there is a body and that we do not know the
length of it yet.

The transports own the "vocabulary", because internally H1 needs
to know the difference between BS_CHUNKED and BS_EOF in order to
setup the VFPs.

bin/varnishd/cache/cache.h

@@ -71,13 +71,6 @@ typedef const struct body_status *body_status_t;
 
 /*--------------------------------------------------------------------*/
 
-enum req_body_state_e {
-#define REQ_BODY(U)	REQ_BODY_##U,
-#include "tbl/req_body.h"
-};
-
-/*--------------------------------------------------------------------*/
-
 enum sess_close {
 	SC_NULL = 0,
 #define SESS_CLOSE(nm, stat, err, desc)	SC_##nm,
@@ -464,7 +457,7 @@ struct req {
 #define REQ_MAGIC		0x2751aaa1
 
 	enum req_step		req_step;
-	volatile enum req_body_state_e	req_body_status;
+	body_status_t		req_body_status;
 	enum sess_close		doclose;
 	unsigned		restarts;
 	unsigned		esi_level;

bin/varnishd/cache/cache_esi_deliver.c

@@ -171,7 +171,7 @@ ved_include(struct req *preq, const char *src, const char *host,
 
 	/* Client content already taken care of */
 	http_Unset(req->http, H_Content_Length);
-	req->req_body_status = REQ_BODY_NONE;
+	req->req_body_status = BS_NONE;
 
 	AZ(req->vcl);
 	AN(req->top);

bin/varnishd/cache/cache_fetch.c

@@ -240,18 +240,15 @@ vbf_stp_mkbereq(struct worker *wrk, struct busyobj *bo)
 	bo->ws_bo = WS_Snapshot(bo->ws);
 	HTTP_Clone(bo->bereq, bo->bereq0);
 
-	if (bo->req->req_body_status == REQ_BODY_NONE) {
+	if (bo->req->req_body_status->avail == 0) {
 		bo->req = NULL;
 		ObjSetState(bo->wrk, bo->fetch_objcore, BOS_REQ_DONE);
-	} else if (bo->req->req_body_status == REQ_BODY_CACHED) {
+	} else if (bo->req->req_body_status == BS_CACHED) {
 		AN(bo->req->body_oc);
 		bo->bereq_body = bo->req->body_oc;
 		HSH_Ref(bo->bereq_body);
 		bo->req = NULL;
 		ObjSetState(bo->wrk, bo->fetch_objcore, BOS_REQ_DONE);
-	} else if (bo->req->req_body_status != REQ_BODY_LENGTH &&
-	    bo->req->req_body_status != REQ_BODY_WITHOUT_LEN) {
-		WRONG("Bad req_body_status");
 	}
 	return (F_STP_STARTFETCH);
 }

bin/varnishd/cache/cache_panic.c

@@ -71,19 +71,6 @@ static void pan_req(struct vsb *, const struct req *);
 
 /*--------------------------------------------------------------------*/
 
-static const char *
-reqbody_status_2str(enum req_body_state_e e)
-{
-	switch (e) {
-#define REQ_BODY(U) case REQ_BODY_##U: return("R_BODY_" #U);
-#include "tbl/req_body.h"
-	default:
-		return ("?");
-	}
-}
-
-/*--------------------------------------------------------------------*/
-
 static const char *
 boc_state_2str(enum boc_state_e e)
 {
@@ -524,7 +511,7 @@ pan_req(struct vsb *vsb, const struct req *req)
 		VSB_printf(vsb, "step = 0x%x,\n", req->req_step);
 
 	VSB_printf(vsb, "req_body = %s,\n",
-	    reqbody_status_2str(req->req_body_status));
+	    req->req_body_status ? req->req_body_status->name : "NULL");
 
 	if (req->err_code)
 		VSB_printf(vsb,

bin/varnishd/cache/cache_req.c

@@ -94,7 +94,6 @@ Req_New(const struct worker *wrk, struct sess *sp)
 	AN(req);
 	req->magic = REQ_MAGIC;
 	req->sp = sp;
-	req->req_body_status = REQ_BODY_INIT;
 
 	e = (char*)req + sz;
 	p = (char*)(req + 1);
@@ -242,7 +241,7 @@ Req_Cleanup(struct sess *sp, struct worker *wrk, struct req *req)
 	req->t_first = NAN;
 	req->t_prev = NAN;
 	req->t_req = NAN;
-	req->req_body_status = REQ_BODY_INIT;
+	req->req_body_status = NULL;
 
 	req->hash_always_miss = 0;
 	req->hash_ignore_busy = 0;

bin/varnishd/cache/cache_req_body.c

@@ -77,7 +77,7 @@ vrb_pull(struct req *req, ssize_t maxsize, objiterate_f *func, void *priv)
 	req->storage = NULL;
 
 	if (STV_NewObject(req->wrk, req->body_oc, stv, 8) == 0) {
-		req->req_body_status = REQ_BODY_ERROR;
+		req->req_body_status = BS_ERROR;
 		HSH_DerefBoc(req->wrk, req->body_oc);
 		AZ(HSH_DerefObjCore(req->wrk, &req->body_oc, 0));
 		(void)VFP_Error(vfc, "Object allocation failed:"
@@ -88,7 +88,7 @@ vrb_pull(struct req *req, ssize_t maxsize, objiterate_f *func, void *priv)
 	vfc->oc = req->body_oc;
 
 	if (VFP_Open(vfc) < 0) {
-		req->req_body_status = REQ_BODY_ERROR;
+		req->req_body_status = BS_ERROR;
 		HSH_DerefBoc(req->wrk, req->body_oc);
 		AZ(HSH_DerefObjCore(req->wrk, &req->body_oc, 0));
 		return (-1);
@@ -136,7 +136,7 @@ vrb_pull(struct req *req, ssize_t maxsize, objiterate_f *func, void *priv)
 		HSH_DerefBoc(req->wrk, req->body_oc);
 		AZ(HSH_DerefObjCore(req->wrk, &req->body_oc, 0));
 		if (vfps != VFP_END) {
-			req->req_body_status = REQ_BODY_ERROR;
+			req->req_body_status = BS_ERROR;
 			if (r == 0)
 				r = -1;
 		}
@@ -148,7 +148,7 @@ vrb_pull(struct req *req, ssize_t maxsize, objiterate_f *func, void *priv)
 	HSH_DerefBoc(req->wrk, req->body_oc);
 
 	if (vfps != VFP_END) {
-		req->req_body_status = REQ_BODY_ERROR;
+		req->req_body_status = BS_ERROR;
 		AZ(HSH_DerefObjCore(req->wrk, &req->body_oc, 0));
 		return (-1);
 	}
@@ -167,7 +167,7 @@ vrb_pull(struct req *req, ssize_t maxsize, objiterate_f *func, void *priv)
 		    (uintmax_t)req_bodybytes);
 	}
 
-	req->req_body_status = REQ_BODY_CACHED;
+	req->req_body_status = BS_CACHED;
 	return (req_bodybytes);
 }
 
@@ -189,32 +189,27 @@ VRB_Iterate(struct worker *wrk, struct vsl_log *vsl,
 	CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
 	AN(func);
 
-	switch (req->req_body_status) {
-	case REQ_BODY_CACHED:
+	if (req->req_body_status == BS_CACHED) {
 		AN(req->body_oc);
 		if (ObjIterate(wrk, req->body_oc, priv, func, 0))
 			return (-1);
 		return (0);
-	case REQ_BODY_NONE:
+	}
+	if (req->req_body_status == BS_NONE)
 		return (0);
-	case REQ_BODY_LENGTH:
-	case REQ_BODY_WITHOUT_LEN:
-		break;
-	case REQ_BODY_TAKEN:
+	if (req->req_body_status == BS_TAKEN) {
 		VSLb(vsl, SLT_VCL_Error,
 		    "Uncached req.body can only be consumed once.");
 		return (-1);
-	case REQ_BODY_ERROR:
+	}
+	if (req->req_body_status == BS_ERROR) {
 		VSLb(vsl, SLT_FetchError,
 		    "Had failed reading req.body before.");
 		return (-1);
-	default:
-		WRONG("Wrong req_body_status in VRB_Iterate()");
 	}
 	Lck_Lock(&req->sp->mtx);
-	if (req->req_body_status == REQ_BODY_LENGTH ||
-	    req->req_body_status == REQ_BODY_WITHOUT_LEN) {
-		req->req_body_status = REQ_BODY_TAKEN;
+	if (req->req_body_status->avail > 0) {
+		req->req_body_status = BS_TAKEN;
 		i = 0;
 	} else
 		i = -1;
@@ -254,9 +249,9 @@ VRB_Ignore(struct req *req)
 
 	if (req->doclose)
 		return (0);
-	if (req->req_body_status == REQ_BODY_LENGTH ||
-	    req->req_body_status == REQ_BODY_WITHOUT_LEN)
-		(void)VRB_Iterate(req->wrk, req->vsl, req, httpq_req_body_discard, NULL);
+	if (req->req_body_status->avail > 0)
+		(void)VRB_Iterate(req->wrk, req->vsl, req,
+		    httpq_req_body_discard, NULL);
 	return (0);
 }
 
@@ -292,6 +287,7 @@ VRB_Cache(struct req *req, ssize_t maxsize)
 	uint64_t u;
 
 	CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
+	assert (req->req_step == R_STP_RECV);
 	assert(maxsize >= 0);
 
 	/*
@@ -299,27 +295,22 @@ VRB_Cache(struct req *req, ssize_t maxsize)
 	 * where we know we will have no competition or conflicts for the
 	 * updates to req.http.* etc.
 	 */
-	if (req->restarts > 0 && req->req_body_status != REQ_BODY_CACHED)
+	if (req->restarts > 0 && req->req_body_status != BS_CACHED) {
+		VSLb(req->vsl, SLT_VCL_Error,
+		    "req.body must be cached before restarts");
 		return (-1);
+	}
 
-	assert (req->req_step == R_STP_RECV);
-	switch (req->req_body_status) {
-	case REQ_BODY_CACHED:
+	if (req->req_body_status == BS_CACHED) {
 		AZ(ObjGetU64(req->wrk, req->body_oc, OA_LEN, &u));
 		return (u);
-	case REQ_BODY_ERROR:
-		return (-1);
-	case REQ_BODY_NONE:
-		return (0);
-	case REQ_BODY_WITHOUT_LEN:
-	case REQ_BODY_LENGTH:
-		break;
-	default:
-		WRONG("Wrong req_body_status in VRB_Cache()");
 	}
 
+	if (req->req_body_status->avail <= 0)
+		return (req->req_body_status->avail);
+
 	if (req->htc->content_length > maxsize) {
-		req->req_body_status = REQ_BODY_ERROR;
+		req->req_body_status = BS_ERROR;
 		(void)VFP_Error(req->vfc, "Request body too big to cache");
 		return (-1);
 	}

bin/varnishd/cache/cache_req_fsm.c

@@ -66,7 +66,7 @@ cnt_transport(struct worker *wrk, struct req *req)
 	CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
 	CHECK_OBJ_NOTNULL(req->http, HTTP_MAGIC);
 	CHECK_OBJ_NOTNULL(req->transport, TRANSPORT_MAGIC);
-	assert(req->req_body_status != REQ_BODY_INIT);
+	AN(req->req_body_status);
 
 	if (http_GetHdr(req->http, H_Expect, &p)) {
 		if (strcasecmp(p, "100-continue")) {
@@ -89,7 +89,7 @@ cnt_transport(struct worker *wrk, struct req *req)
 		return (REQ_FSM_DONE);
 	}
 
-	if (req->req_body_status < REQ_BODY_TAKEN) {
+	if (req->req_body_status->avail == 1) {
 		AN(req->transport->req_body != NULL);
 		VFP_Setup(req->vfc, wrk);
 		req->vfc->resp = req->http;		// XXX
@@ -741,8 +741,8 @@ cnt_pipe(struct worker *wrk, struct req *req)
 			bo->req = req;
 			bo->wrk = wrk;
 			/* Unless cached, reqbody is not our job */
-			if (req->req_body_status != REQ_BODY_CACHED)
-				req->req_body_status = REQ_BODY_NONE;
+			if (req->req_body_status != BS_CACHED)
+				req->req_body_status = BS_NONE;
 			SES_Close(req->sp, VDI_Http1Pipe(req, bo));
 			nxt = REQ_FSM_DONE;
 			V1P_Leave();
@@ -877,7 +877,7 @@ cnt_recv(struct worker *wrk, struct req *req)
 
 	cnt_recv_prep(req, ci);
 
-	if (req->req_body_status == REQ_BODY_ERROR) {
+	if (req->req_body_status == BS_ERROR) {
 		req->doclose = SC_OVERLOAD;
 		return (REQ_FSM_DONE);
 	}
@@ -899,7 +899,7 @@ cnt_recv(struct worker *wrk, struct req *req)
 	}
 
 	/* Attempts to cache req.body may fail */
-	if (req->req_body_status == REQ_BODY_ERROR) {
+	if (req->req_body_status == BS_ERROR) {
 		req->doclose = SC_RX_BODY;
 		return (REQ_FSM_DONE);
 	}

bin/varnishd/http1/cache_http1_fetch.c

@@ -91,8 +91,7 @@ V1F_SendReq(struct worker *wrk, struct busyobj *bo, uint64_t *ctr_hdrbytes,
 	assert(*htc->rfd > 0);
 	hp = bo->bereq;
 
-	if (bo->req != NULL &&
-	    bo->req->req_body_status == REQ_BODY_WITHOUT_LEN) {
+	if (bo->req != NULL && !bo->req->req_body_status->length_known) {
 		http_PrintfHeader(hp, "Transfer-Encoding: chunked");
 		do_chunked = 1;
 	}
@@ -112,15 +111,15 @@ V1F_SendReq(struct worker *wrk, struct busyobj *bo, uint64_t *ctr_hdrbytes,
 		(void)ObjIterate(bo->wrk, bo->bereq_body,
 		    bo, vbf_iter_req_body, 0);
 	} else if (bo->req != NULL &&
-	    bo->req->req_body_status != REQ_BODY_NONE) {
+	    bo->req->req_body_status != BS_NONE) {
 		if (do_chunked)
 			V1L_Chunked(wrk);
 		i = VRB_Iterate(wrk, bo->vsl, bo->req, vbf_iter_req_body, bo);
 
-		if (bo->req->req_body_status != REQ_BODY_CACHED)
+		if (bo->req->req_body_status != BS_CACHED)
 			bo->no_retry = "req.body not cached";
 
-		if (bo->req->req_body_status == REQ_BODY_ERROR) {
+		if (bo->req->req_body_status == BS_ERROR) {
 			/*
 			 * XXX: (#2332) We should test to see if the backend
 			 * XXX: sent us some headers explaining why.

bin/varnishd/http1/cache_http1_fsm.c

@@ -151,9 +151,8 @@ http1_req_body(struct req *req)
 {
 
 	CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
-	if (req->htc->body_status->avail &&
-	    V1F_Setup_Fetch(req->vfc, req->htc) != 0)
-		req->req_body_status = REQ_BODY_ERROR;
+	if (V1F_Setup_Fetch(req->vfc, req->htc) != 0)
+		req->req_body_status = BS_ERROR;
 }
 
 static void
@@ -273,18 +272,8 @@ http1_dissect(struct worker *wrk, struct req *req)
 		return (-1);
 	}
 
-	assert (req->req_body_status == REQ_BODY_INIT);
-
-	if (req->htc->body_status == BS_CHUNKED)
-		req->req_body_status = REQ_BODY_WITHOUT_LEN;
-	else if (req->htc->body_status == BS_LENGTH)
-		req->req_body_status = REQ_BODY_LENGTH;
-	else if (req->htc->body_status == BS_NONE)
-		req->req_body_status = REQ_BODY_NONE;
-	else if (req->htc->body_status == BS_EOF)
-		req->req_body_status = REQ_BODY_WITHOUT_LEN;
-	else
-		WRONG("Unknown req_body_status situation");
+	AZ(req->req_body_status);
+	req->req_body_status = req->htc->body_status;
 	return (0);
 }
 

bin/varnishd/http2/cache_http2_proto.c

@@ -559,11 +559,6 @@ h2_end_headers(struct worker *wrk, struct h2_sess *h2,
 	assert(r2->state == H2_S_OPEN);
 	h2e = h2h_decode_fini(h2);
 	h2->new_req = NULL;
-	if (r2->req->req_body_status == REQ_BODY_NONE) {
-		/* REQ_BODY_NONE implies one of the frames in the
-		 * header block contained END_STREAM */
-		r2->state = H2_S_CLOS_REM;
-	}
 	if (h2e != NULL) {
 		Lck_Lock(&h2->sess->mtx);
 		VSLb(h2->vsl, SLT_Debug, "HPACK/FINI %s", h2e->name);
@@ -579,13 +574,15 @@ h2_end_headers(struct worker *wrk, struct h2_sess *h2,
 	// XXX: Have I mentioned H/2 Is hodge-podge ?
 	http_CollectHdrSep(req->http, H_Cookie, "; ");	// rfc7540,l,3114,3120
 
-	if (req->req_body_status == REQ_BODY_INIT) {
+	if (req->req_body_status == NULL) {
 		if (!http_GetHdr(req->http, H_Content_Length, NULL))
-			req->req_body_status = REQ_BODY_WITHOUT_LEN;
+			req->req_body_status = BS_EOF;
 		else
-			req->req_body_status = REQ_BODY_LENGTH;
+			req->req_body_status = BS_LENGTH;
 	} else {
-		assert (req->req_body_status == REQ_BODY_NONE);
+		/* A HEADER frame contained END_STREAM */
+		assert (req->req_body_status == BS_NONE);
+		r2->state = H2_S_CLOS_REM;
 		if (http_GetContentLength(req->http) > 0)
 			return (H2CE_PROTOCOL_ERROR); //rfc7540,l,1838,1840
 	}
@@ -694,7 +691,7 @@ h2_rx_headers(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2)
 	}
 
 	if (h2->rxf_flags & H2FF_HEADERS_END_STREAM)
-		req->req_body_status = REQ_BODY_NONE;
+		req->req_body_status = BS_NONE;
 
 	if (h2->rxf_flags & H2FF_HEADERS_END_HEADERS)
 		return (h2_end_headers(wrk, h2, req, r2));

include/Makefile.am

@@ -26,7 +26,6 @@ nobase_pkginclude_HEADERS = \
 	tbl/oc_exp_flags.h \
 	tbl/oc_flags.h \
 	tbl/params.h \
-	tbl/req_body.h \
 	tbl/req_flags.h \
 	tbl/sess_attr.h \
 	tbl/sess_close.h \

include/tbl/body_status.h

@@ -34,10 +34,12 @@
 
 /*         Upper        lower         nbr,	avail   len_known */
 BODYSTATUS(NONE,	none,		0,	0,	1)
-BODYSTATUS(ERROR,	error,		1,	0,	0)
+BODYSTATUS(ERROR,	error,		1,	-1,	0)
 BODYSTATUS(CHUNKED,	chunked,	2,	1,	0)
 BODYSTATUS(LENGTH,	length,		3,	1,	1)
 BODYSTATUS(EOF,		eof,		4,	1,	0)
+BODYSTATUS(TAKEN,	taken,		5,	0,	0)
+BODYSTATUS(CACHED,	cached,		6,	2,	1)
 #undef BODYSTATUS
 
 /*lint -restore */

include/tbl/req_body.h

-/*-
- * Copyright (c) 2013-2015 Varnish Software AS
- * All rights reserved.
- *
- * Author: Poul-Henning Kamp <phk@phk.freebsd.dk>
- *
- * SPDX-License-Identifier: BSD-2-Clause
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- *    notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- *    notice, this list of conditions and the following disclaimer in the
- *    documentation and/or other materials provided with the distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
- * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
- * ARE DISCLAIMED.  IN NO EVENT SHALL AUTHOR OR CONTRIBUTORS BE LIABLE
- * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
- * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
- * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
- * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
- * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
- * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
- * SUCH DAMAGE.
- *
- */
-
-/*lint -save -e525 -e539 */
-
-REQ_BODY(INIT)
-REQ_BODY(WITHOUT_LEN)
-REQ_BODY(LENGTH)
-/* states >= TAKEN imply that no body is to be read */
-REQ_BODY(TAKEN)
-REQ_BODY(CACHED)
-REQ_BODY(ERROR)
-REQ_BODY(NONE)
-#undef REQ_BODY
-
-/*lint -restore */