Mailing List Archive

commit 532e9c207c93c954807422e3c748ff668b140333
Author: Nils Goroll <nils.goroll@uplex.de>
Date: Sun Dec 31 17:09:17 2023 +0100

Clarify and fix STV_NewObject() len/wsl argument use

It was not documented explicitly anywhere, but the len/wsl argument to
STV_NewObject() is the amount of space to allocate for all
OBJ_VARATTRs combined which are going to be written to this
object. For now, these are OA_HEADERS and OA_VARY only (see
include/tbl/obj_attr.h).

So for one, there is no reason to force this argument be greater than
zero when we know that no OBJ_VARATTRs are going to be set.

Secondly, this might actually represent a minor shortcoming of the
stevedore API, because the amount of space which a stevedore
implementation actually needs might be larger than what the simple
stevedores use. So for now, the semantics of this argument are, more
specifically, "the equivalent of space for OBJ_VARATTRs as for simple
storage". In practice, this probably does not matter much...

But even before this clarification, the API was not used consistently:

For the call from vrb_pull(), the maximum request body size (to cache)
was used as the wsl argument, yet it has nothing to do with the object
body size, it specifies the amount of space to allocate for variable
sized object attributes (see above).

For the call from cnt_synth(), a somehow arbitrary value of 1KB was
used.

In both cases, the amount of space actually required is zero, because
the only attribute used on the objects created is OA_LEN, which is
fixed and thus always present.

diff --git a/bin/varnishd/cache/cache_req_body.c b/bin/varnishd/cache/cache_req_body.c
index 3cbd62734..70996a2f1 100644
--- a/bin/varnishd/cache/cache_req_body.c
+++ b/bin/varnishd/cache/cache_req_body.c
@@ -59,7 +59,6 @@ vrb_pull(struct req *req, ssize_t maxsize, objiterate_f *func, void *priv)
enum vfp_status vfps = VFP_ERROR;
const struct stevedore *stv;
ssize_t req_bodybytes = 0;
- unsigned hint;

CHECK_OBJ_NOTNULL(req, REQ_MAGIC);

@@ -77,8 +76,7 @@ vrb_pull(struct req *req, ssize_t maxsize, objiterate_f *func, void *priv)

req->storage = NULL;

- hint = maxsize > 0 ? maxsize : 1;
- if (STV_NewObject(req->wrk, req->body_oc, stv, hint) == 0) {
+ if (STV_NewObject(req->wrk, req->body_oc, stv, 0) == 0) {
req->req_body_status = BS_ERROR;
HSH_DerefBoc(req->wrk, req->body_oc);
AZ(HSH_DerefObjCore(req->wrk, &req->body_oc, 0));
diff --git a/bin/varnishd/cache/cache_req_fsm.c b/bin/varnishd/cache/cache_req_fsm.c
index bb7a43d4e..c7469f5eb 100644
--- a/bin/varnishd/cache/cache_req_fsm.c
+++ b/bin/varnishd/cache/cache_req_fsm.c
@@ -383,7 +383,7 @@ cnt_synth(struct worker *wrk, struct req *req)
req->objcore = HSH_Private(wrk);
CHECK_OBJ_NOTNULL(req->objcore, OBJCORE_MAGIC);
szl = -1;
- if (STV_NewObject(wrk, req->objcore, stv_transient, 1024)) {
+ if (STV_NewObject(wrk, req->objcore, stv_transient, 0)) {
body = VSB_data(synth_body);
szl = VSB_len(synth_body);
assert(szl >= 0);
diff --git a/bin/varnishd/cache/cache_varnishd.h b/bin/varnishd/cache/cache_varnishd.h
index 404ebc894..062041f49 100644
--- a/bin/varnishd/cache/cache_varnishd.h
+++ b/bin/varnishd/cache/cache_varnishd.h
@@ -564,6 +564,7 @@ const struct stevedore *STV_next(void);
int STV_BanInfoDrop(const uint8_t *ban, unsigned len);
int STV_BanInfoNew(const uint8_t *ban, unsigned len);
void STV_BanExport(const uint8_t *banlist, unsigned len);
+// STV_NewObject() len is space for OBJ_VARATTR
int STV_NewObject(struct worker *, struct objcore *,
const struct stevedore *, unsigned len);

diff --git a/bin/varnishd/storage/stevedore.c b/bin/varnishd/storage/stevedore.c
index 9c2847997..7813c424f 100644
--- a/bin/varnishd/storage/stevedore.c
+++ b/bin/varnishd/storage/stevedore.c
@@ -84,7 +84,6 @@ STV_NewObject(struct worker *wrk, struct objcore *oc,
CHECK_OBJ_NOTNULL(oc, OBJCORE_MAGIC);
CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
CHECK_OBJ_NOTNULL(stv, STEVEDORE_MAGIC);
- assert(wsl > 0);

wrk->strangelove = cache_param->nuke_limit;
AN(stv->allocobj);
_______________________________________________
varnish-commit mailing list
varnish-commit@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-commit