[PATCH] backend conditional requests first release

[Geoff Simmons]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 3/1/11 8:00 PM, Dmitry Panov wrote:
>
> Thanks for the patch. Unfortunately it does not compile for me:
>
> gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I../.. -I../../include
> -I../../lib/libvgz -I../../include
> -DVARNISH_STATE_DIR='"/opt/varnish/var/varnish"'
> -DVARNISH_VMOD_DIR='"/opt/varnish/lib/varnish/vmods"'
> -DVARNISH_VCL_DIR='"/opt/varnish/etc/varnish"' -g -O2 -pthread -Wall
> -Wstrict-prototypes -Wmissing-prototypes -Wpointer-arith -Wreturn-type
> -Wcast-qual -Wwrite-strings -Wswitch -Wshadow -Wcast-align
> -Wunused-parameter -Wchar-subscripts -Winline -Wnested-externs
> -Wredundant-decls -Wformat -O0 -g -fno-inline -DDIAGNOSTICS -Wextra
> -Wno-missing-field-initializers -Wno-sign-compare -fstack-protector-all
> -Werror -MT varnishd-cache_vrt_var.o -MD -MP -MF
> .deps/varnishd-cache_vrt_var.Tpo -c -o varnishd-cache_vrt_var.o `test -f
> 'cache_vrt_var.c' || echo './'`cache_vrt_var.c
> cc1: warnings being treated as errors
> cache_vrt_var.c:112: error: no previous prototype for
> ‘VRT_l_stale_obj_proto’
> cache_vrt_var.c:113: error: no previous prototype for
> ‘VRT_l_stale_obj_response’
> cache_vrt_var.c:143: error: no previous prototype for
> ‘VRT_l_stale_obj_status’
> cache_vrt_var.c:388: error: no previous prototype for ‘vrt_r_obj_ttl’
> cache_vrt_var.c:416: error: no previous prototype for
> ‘vrt_r_obj_conditional_timeout’
> cache_vrt_var.c:548: error: no previous prototype for
> ‘VRT_l_stale_obj_grace’
> make[3]: *** [varnishd-cache_vrt_var.o] Error 1

You need to compile the patch without -Werror, i.e. without
- --enable-werror in the configure invocation.

This brings up an implementation issue, a little complex and probably
only interesting to hackers.

We've added stale_obj to VCL, and in cache_vrt_var.c, we tried to avoid
duplicating code by re-using macros and functions for accessing fields
of stale_obj that other VCL objects also have. For example, we adapted
VRT_DO_HDR() to implement access to stale_obj.response and
stale_obj.proto, as it does for req, bereq and so on.

But that doesn't seem to be working out, because unlike the other
objects, stale_obj may be NULL, and it is read-only. So in the read
functions, it has to be checked for NULL, and if so, a warning is issued
and a "default" value is returned.

The other objects may never be NULL, and that was enforced with
CHECK_OBJ_NOTNULL. We had to remove this check, check for NULL, call
CHECK_OBJ and return the value if it isn't, and return the "default"
value if it is. But since the macro is used for all of the other VCL
objects, they aren't being enforced as non-NULL any more.

Also, a macro like VRT_DO_HDR() generates the "left-hand" functions for
write access, so these functions get generated for stale_obj as well,
even though write access to stale_obj is syntactically illegal.

This was the cause of Dmitry's problem. A function like
VRT_l_stale_obj_proto() is generated in cache_vrt_var.c by VRT_DO_HDR(),
but its prototype is not generated by generate.py. A missing prototype
is a warning, -Werror makes it an error, so the compile fails.

Without -Werror, an implementation for VRT_l_stale_obj_proto() is
created in the shared object. It never gets called, since the VCL
compiler prevents write access to stale_obj, it just takes up space.

It may be possible to write even more complex macros with more
parameters like "nullable" and "writeable", with a lot of #if's that
depend on them. But I'm beginning to think that it would be better to
live with a little duplication and let have stale_obj have its own
functions.

BTW, I couldn't work with -Werror switched on, because there's some
other code in the current trunk that won't compile with it.


Best,
Geoff
- -- 
UPLEX Systemoptimierung
Schwanenwik 24
22087 Hamburg
http://uplex.de/
Mob: +49-176-63690917
-----BEGIN PGP SIGNATURE-----
Version: GnuPG/MacGPG2 v2.0.14 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBCAAGBQJNbXloAAoJEOUwvh9pJNURCCAQALGDO0CLB+J6Gpee2Vk5/bpK
8ZtzTmJGhB30WxBORoscfMQmkeE4BaU+b3EroQRCMbjE4XSBDILBoLxdYBmtIudE
xh69Eqf9V88KIHnGwpxEr5A9fFpwGau35ZQTCFR1E0tznWCrnTcjkvb7SkfTjBPH
p4t5XC9hEHXjeXSyWM5jniLvoRJACGARniIB7fCosPejhDsWAM1UFK6jbm99B/Nl
THWTdTEW2I38oqVtGXcCZR4suVd9/ZBwfWCtiq1cb2rkWftOWtDFxQMXs///brua
iDlbV0Wzi2Vs9Plfxdvnrh1yz/50Jbq1w0H9thiEQRogPhRttcMK8v//qtGHBowk
RpfETUrHcPgs/h1qvQW7PoR3BED0BKiY1dZvVd1oowys6oTV+7f+RpBUYVpTrVoB
S3jCT8VWyvEIUyb3H5n76dz2ICL7oX/r3u1Hhs4p6V+YHIgVGhoUrs59e+/OxRgz
FXb3jj1JOPI8ZKcpt+kQGaXcP6eU+ot1Zx1EWDEd0XxR+ZURHpffY3ooGNIR+pwg
rZDS+9p8X837L/8LXteU/zXSaRn6lzOr1XRk8vsK9y7SbwCK7yBdYrAHbnw60dOG
xMxkLi3NaHgp4s3+6QZxOpNwi0ecpP+P+YJ1OuX4zA/0dGe57KFbopmT+hTZLUpZ
HiJZr/ANMKc2ylOqCPok
=2qXh
-----END PGP SIGNATURE-----