- 02 Feb, 2021 2 commits
-
-
Poul-Henning Kamp authored
Fixes #3509
-
Poul-Henning Kamp authored
-
- 01 Feb, 2021 4 commits
-
-
Nils Goroll authored
We wrap VAS_Fail() as VPI_Fail() and add an assert() macro to the Varnish Generated C (VGC) code. This assert() definition is basically a copy of that in vas.h, but deliberately added to a source file which is only used by generate.py for VGC, such that this slightly different definition be only visible to VGC. I also pondered the option to include VCL source information in VPI_Fail: By using the information updated by VPI_count(), we could have that, but as the assert is, for now, intended only to ensure correctness of VCC and core VRT code, I decided against this complication. With assert() in place for VGC, we arm the assertions from 75acb5cc
-
Nils Goroll authored
It seems they do not parse attributes and how I read --macro-file-builtins they just define them to NIL.
-
Nils Goroll authored
How 16, you ask? Consider alignment...
-
Dridi Boukelmoune authored
One of the VCL 4.1 changes was to make the protocol read-only, which we've successfully done for all variables except resp.proto that kept the exact same definition for both 4.0 and 4.1 versions, despite having distinct definitions.
-
- 31 Jan, 2021 2 commits
-
-
Nils Goroll authored
-
Nils Goroll authored
How I added generation of the assertion code was motivated by how I wanted to lay out more changes in that area later, but generating parts of the function body before the header turned out to not help clarity. Ref 75acb5cc
-
- 30 Jan, 2021 2 commits
-
-
Nils Goroll authored
Bail out upon first error, as for the non-recursive case.
-
Nils Goroll authored
in the course of #3163 we want to add a preamble to VCL subs, which, ideally, would perform as many checks based on constants as possible. This is a first step towards such checks: We add a commented-out assert() on ctx->method for built-in subs only. To add the same for custom subs, we will need the okmask from #3163, for which I would first want to coordinate with phk. Also, I would want to coordinate with him on arming the assertion, as there might be a good reason why assert() is not yet defined for vgc (it is already used in unused macros, though). Having assertions would help humans, flexelint and compilers alike. Until armed, the assertion helps humans at least. The resulting vgc looks like this: void v_matchproto_(vcl_func_f) VGC_function_vcl_backend_error(VRT_CTX) { // assert(ctx->method == (VCL_MET_BACKEND_ERROR)); /* ... from ('Builtin' Line 165 Pos 23) */
-
- 29 Jan, 2021 3 commits
-
-
Poul-Henning Kamp authored
-
Poul-Henning Kamp authored
Add a VCL_BLOB for a connection preamble to struct vrt_backend (mortally offending FlexeLint in the process.)
-
Poul-Henning Kamp authored
-
- 28 Jan, 2021 3 commits
-
-
Nils Goroll authored
Quote more. Do not quote too much Ref da2409c3
-
Nils Goroll authored
We have now seen a second case after 184736fa, and a simple internal rename from bool to boolean does the job. Should have no user-visible impact.
-
Nils Goroll authored
In fee476ac VRB_Ignore(), VRB_Cache() and VRB_Free() were rightly out of vmods' sight. But VRB_Iterate() is legitemately required for vmods, and including it in the move seems to be an accident.
-
- 27 Jan, 2021 1 commit
-
-
Nils Goroll authored
Advise from shellcheck taken undogmatically. (I always forget: is $() for `..` posix-sh or not?)
-
- 26 Jan, 2021 2 commits
-
-
Poul-Henning Kamp authored
information to make them easier to clone.
-
Dridi Boukelmoune authored
Salvaged from #3503
-
- 25 Jan, 2021 6 commits
-
-
Dridi Boukelmoune authored
The rationale is that if we add another type of task in the future, for example probe tasks, we won't need to update the default mask for VMOD functions and object methods.
-
Dridi Boukelmoune authored
Refs #3498
-
Dridi Boukelmoune authored
With a default read mask based on the type of symbol. Starting with a default comprehensive mask should then help implement the $Restrict stanza from VIP4, simply by AND-ing the masks.
-
Nils Goroll authored
Simply take an ssize_t and return size_t asserting that the value is greater than or equal to zero. The name might still not be optimimal, as well as choice of *size_t. Addresses Coverity CID 1472401, CID 1472402
-
Dridi Boukelmoune authored
This way every failure can be tested again without selecting a specific subdirectory.
-
Nils Goroll authored
adds a variant of PAN_dump_struct() which does not start another indentation level on a new line.
-
- 23 Jan, 2021 4 commits
-
-
Nils Goroll authored
A copy-pasta in 55dffc35 prevented the top request from being dumped.
-
Nils Goroll authored
55dffc35 introduced a PAN_dump_struct() call, which implies printing '{' and VSB_indent(vsb, 2). Fix missing '}' and wrong indentation. Also remove redundant "priv" output, which now happens in PAN_dump_struct() Diff best viewed ignoring whitespace (-b option).
-
Nils Goroll authored
This test fails constantly now on vtest arm / Freebsd 13.0 for timeout. As vcl compilations are probably the most expensive operations of vtcs, we speed up this test by reducing the number of vcls loaded from 19 to 11. We do this by collapsing all of the client side failure tests into a single vcl. Being at it, we also reduce logexpect synchronization points. The number of vcl loads could be reduced further by also collapsing the backend side, but as this would reduce clarity more than for the client side alone, I first wanted to see if this first step brings down tuntime enough.
-
Nils Goroll authored
Do not overwrite an already latched errno. Note that it does not make any difference at the moment because we only ever use ENOMEM Ref #3502
-
- 22 Jan, 2021 3 commits
-
-
Nils Goroll authored
This is similar to #3502, but the out-of-storage condition happens after the object has been created successfully. Use a logexpect to ensure we test as intended.
-
Nils Goroll authored
We set up fetch filters (VFPs) before initializing a storage object to fetch into. If that fails, we close the filters again. The esi_gzip VFP interacts with the storage object and was missing error handling for the case of a teardown with no storage object. Implementation: We make sure that for the VFP_ERROR case, we do not interact with the storage object (which makes no sense if that is going to be C-4'ed any moment). vfp_vep_callback() keeps its hands off if (struct vef_priv).error is set, so we use that to avoid complications in the code.
-
Nils Goroll authored
-
- 21 Jan, 2021 1 commit
-
-
Nils Goroll authored
... which is not important enough for me to tackle it right now, in particular because I really do not understand enough about how this code works yet, e.g. which of the if() blocks can possibly happen in which order. *cough* fsm? *cough*
-
- 20 Jan, 2021 6 commits
-
-
Dridi Boukelmoune authored
This was already the behavior before #3468, but then the filtering was moved to libvarnishapi to prevent varnishncsa from duplicating the logic behind transaction collection. Unfortunately, despite libvarnishapi's effort to not consider a client or backend transaction without a Begin tag as a candidate, a transaction missing one will get a synthetic Begin prior to dispatch. Regardless, that would have only applied to vxid grouping so even if varnishncsa was presented by default with only relevant transactions, it could still see incomplete transactions in request grouping panic for the same reason. Fixes #3501
-
Nils Goroll authored
do someone a favor and reformat the VPI_vcl_fini() call. Because ifp->fin basically gets output via "\t%s\n", this triggers my OCD in a some other way ;), but I am not going down that rabbit hole at this point...
-
Nils Goroll authored
We do not need to call trunc() to truncate a double into an integert. Mark the intention for flexelint with a cast.
-
Nils Goroll authored
This change is based on the precondition that VRT_fail() should work wherever we have a VRT_CTX and DTRT, see also previous. on vcl_init {} -------------- We already handled a "direct" VRT_fail() via a vmod function correctly. This commit adds a test. Yet we can also VRT_fail() via a PRIV_TASK fini callback and did not handle that case (triggered an assertion in VRT_fail()). Notice that this case has worked in client / backend tasks for long and a test case has been added with 746384b2 . It has now gained relevance with 43d9e5fb on vcl_fini {} -------------- By design, vcl_fini {} must not fail, which is also why the only valid return() value is "ok" (VCL_RET_OK). Consequently, we had if (method && *ctx->handling && *ctx->handling != VCL_RET_OK) { assert(ev == VCL_EVENT_LOAD); in vcl_send_event(). And then came VRT_fail(). If we wanted to sustain an assertion similar to the above, we would need to require vmod authors to never call VRT_fail() from VCL_MET_FINI - which would complicate code, be likely overlooked and receive little to no test coverage. So instead we now ignore errors during vcl_fini {} and emit a VCL_error log line. I considered the alternative to check for VCL_MET_FINI in VRT_fail(), but decided to handle the exception where it applies, which is in vcl_send_event(). I am aware that this part of the change may trigger some controversy and I am open to discussing alternatives. If anything, we now avoid unmotivated assertion failures triggered for the new tests in v00051.vtc for the time being.
-
Nils Goroll authored
Ref 43d9e5fb : PRIV_* fini methods need to leave (struct vrt_ctx).handling alone, except that they might call VRT_fail(), see also 746384b2 Thus we add assertions that handling be either 0 or VCL_RET_FAIL outside the FSM. To be able to do so, we need to change VCL_RET_OK into 0 when vcl_init{} has returned successfully. The vcl_fini{} case is slightly more complicated: By design, only "ok" (VCL_RET_OK) is allowed, but VRT_fail() also added VCL_RET_FAIL, so we de-facto get a "fail" return if any vmod code called VRT_fail(). Because PRIV_* handling happens from VCC generated code via VGC_Discard(), we need to check and change (struct vrt_ctx).handling right after calling vcl_fini{} / VGC_function_vcl_fini() from VGC_Discard(). This is VPI_vcl_fini(). Implementation note: I also considered void VPI_vcl_fini(VRT_CTX, vcl_func_f fini_sub), having VPI_vcl_fini call the fini_sub, but that stirred up includes of VPI where vcl.h is not included.
-
Nils Goroll authored
Though introduced for the purpose of failing upon rollback, they actually just call VRT_fail() from the PRIV_* fini callback and thus should be named accordingly.
-
- 19 Jan, 2021 1 commit
-
-
Poul-Henning Kamp authored
Not sure where things went wrong on this...
-