Re: Move re test method into libvcl
On 13 Apr 2008, at 08:03, Poul-Henning Kamp wrote: > In message <[EMAIL PROTECTED]>, > Paul Nasrat w > rites: >> >> On 12 Apr 2008, at 19:19, Poul-Henning Kamp wrote: >> >>> Your patch breaks the convention that compiled code only calls VRT_* >>> functions, you need to keep a VRT_re_test() wrapper around for that >>> reason. >> >> I'm not sure that's breaking that convention - the call chain is in >> vcc_regexp and it's a test before generating the C for the compiled >> vcl binary. Maybe I'm misunderstanding what you mean by "compiled >> code" in this case - I think that you mean that any generated C from >> vcl should only call VRT_* functions, which this patch doesn't >> change, >> as it's a validity check before code generation occurs. > > Correct, but your patch removes a VRT_ function which the generated > code needs in order to run... I had a look and if we just rerun vcc_gen_fixed_token.tcl after the patch it does the right thing and, there is nothing in the vcl code that generates that function - it was purely used at compile time not run time. After doing this if I look for any references to the VRT_re_test call there are none. I don't think this method was generated, just grouped in with the other regex code but only used in vcc_string.c I'm happy to come up with a patch that keeps a wrapper, but from my reading of the code I'm not sure in this instance it is required. >> I looked on http://varnish.projects.linpro.no/wiki/DeveloperResources >> but didn't see a mention of this convention - is there another >> place I >> should be looking so that I can understand the coding conventions of >> the project? > > They're not well documented, sorry. I'll try to put a wiki page together with what I've learned about vcl internals (adding/removing stuff, etc). Paul ___ varnish-dev mailing list varnish-dev@projects.linpro.no http://projects.linpro.no/mailman/listinfo/varnish-dev
Re: Move re test method into libvcl
In message <[EMAIL PROTECTED]>, Paul Nasrat w rites: > >On 12 Apr 2008, at 19:19, Poul-Henning Kamp wrote: > >> Your patch breaks the convention that compiled code only calls VRT_* >> functions, you need to keep a VRT_re_test() wrapper around for that >> reason. > >I'm not sure that's breaking that convention - the call chain is in >vcc_regexp and it's a test before generating the C for the compiled >vcl binary. Maybe I'm misunderstanding what you mean by "compiled >code" in this case - I think that you mean that any generated C from >vcl should only call VRT_* functions, which this patch doesn't change, >as it's a validity check before code generation occurs. Correct, but your patch removes a VRT_ function which the generated code needs in order to run... >I looked on http://varnish.projects.linpro.no/wiki/DeveloperResources >but didn't see a mention of this convention - is there another place I >should be looking so that I can understand the coding conventions of >the project? They're not well documented, sorry. -- Poul-Henning Kamp | UNIX since Zilog Zeus 3.20 [EMAIL PROTECTED] | TCP/IP since RFC 956 FreeBSD committer | BSD since 4.3-tahoe Never attribute to malice what can adequately be explained by incompetence. ___ varnish-dev mailing list varnish-dev@projects.linpro.no http://projects.linpro.no/mailman/listinfo/varnish-dev
Re: Move re test method into libvcl
On 12 Apr 2008, at 19:19, Poul-Henning Kamp wrote: > Your patch breaks the convention that compiled code only calls VRT_* > functions, you need to keep a VRT_re_test() wrapper around for that > reason. I'm not sure that's breaking that convention - the call chain is in vcc_regexp and it's a test before generating the C for the compiled vcl binary. Maybe I'm misunderstanding what you mean by "compiled code" in this case - I think that you mean that any generated C from vcl should only call VRT_* functions, which this patch doesn't change, as it's a validity check before code generation occurs. I looked on http://varnish.projects.linpro.no/wiki/DeveloperResources but didn't see a mention of this convention - is there another place I should be looking so that I can understand the coding conventions of the project? Paul ___ varnish-dev mailing list varnish-dev@projects.linpro.no http://projects.linpro.no/mailman/listinfo/varnish-dev
Move re test method into libvcl
I was wanting to start looking at the vcl parser, and noticed that you can't link against just libvcl to call VCC_CompileFile due to VRT_re_test being called in vcc_string.c. Although varnishd supports -C to parse only The attached patch against trunk moves and renames the re_test method into vcc_string.c so libvcl could be used outside of varnishd. Paul varnish-move-re-test.patch Description: Binary data ___ varnish-dev mailing list varnish-dev@projects.linpro.no http://projects.linpro.no/mailman/listinfo/varnish-dev
Re: Move re test method into libvcl
In message <[EMAIL PROTECTED]>, Paul Nasrat w
rites:
>> The attached patch against trunk moves and renames the re_test
>> method into vcc_string.c so libvcl could be used outside of varnishd.
>-int
>-VRT_re_test(struct vsb *sb, const char *re, int sub)
>-{
Your patch breaks the convention that compiled code only calls VRT_*
functions, you need to keep a VRT_re_test() wrapper around for that
reason.
--
Poul-Henning Kamp | UNIX since Zilog Zeus 3.20
[EMAIL PROTECTED] | TCP/IP since RFC 956
FreeBSD committer | BSD since 4.3-tahoe
Never attribute to malice what can adequately be explained by incompetence.
___
varnish-dev mailing list
varnish-dev@projects.linpro.no
http://projects.linpro.no/mailman/listinfo/varnish-dev
Re: Move re test method into libvcl
On 12 Apr 2008, at 16:31, Paul Nasrat wrote: I was wanting to start looking at the vcl parser, and noticed that you can't link against just libvcl to call VCC_CompileFile due to VRT_re_test being called in vcc_string.c. Although varnishd supports -C to parse only The attached patch against trunk moves and renames the re_test method into vcc_string.c so libvcl could be used outside of varnishd. Oops - that patch didn't remove the old method definition. Here is an updated version. varnish-move-re-test.patch Description: Binary data ___ varnish-dev mailing list varnish-dev@projects.linpro.no http://projects.linpro.no/mailman/listinfo/varnish-dev
