Mailing List Archive

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: varnish-move-re-test.patch
Type: application/octet-stream
Size: 2272 bytes
Desc: not available
Url : http://projects.linpro.no/pipermail/varnish-dev/attachments/20080412/7ee537ff/attachment.obj
-------------- next part --------------

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: varnish-move-re-test.patch
Type: application/octet-stream
Size: 2272 bytes
Desc: not available
Url : http://projects.linpro.no/pipermail/varnish-dev/attachments/20080412/7ee537ff/attachment-0001.obj
-------------- next part --------------

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.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: varnish-move-re-test.patch
Type: application/octet-stream
Size: 2751 bytes
Desc: not available
Url : http://projects.linpro.no/pipermail/varnish-dev/attachments/20080412/5075a862/attachment-0001.obj
-------------- next part --------------

In message <D4F56D4B-8E7C-4729-A7FB-161CCD681509 at googlemail.com>, 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
phk at FreeBSD.ORG | TCP/IP since RFC 956
FreeBSD committer | BSD since 4.3-tahoe
Never attribute to malice what can adequately be explained by incompetence.

In message <D4F56D4B-8E7C-4729-A7FB-161CCD681509 at googlemail.com>, 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
phk at FreeBSD.ORG | TCP/IP since RFC 956
FreeBSD committer | BSD since 4.3-tahoe
Never attribute to malice what can adequately be explained by incompetence.

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

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

In message <F39ABE22-D50C-429E-9A38-FF905A80DD3F at googlemail.com>, 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
phk at FreeBSD.ORG | TCP/IP since RFC 956
FreeBSD committer | BSD since 4.3-tahoe
Never attribute to malice what can adequately be explained by incompetence.

In message <F39ABE22-D50C-429E-9A38-FF905A80DD3F at googlemail.com>, 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
phk at FreeBSD.ORG | TCP/IP since RFC 956
FreeBSD committer | BSD since 4.3-tahoe
Never attribute to malice what can adequately be explained by incompetence.

On 13 Apr 2008, at 08:03, Poul-Henning Kamp wrote:
> In message <F39ABE22-D50C-429E-9A38-FF905A80DD3F at googlemail.com>,
> 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

On 13 Apr 2008, at 08:03, Poul-Henning Kamp wrote:
> In message <F39ABE22-D50C-429E-9A38-FF905A80DD3F at googlemail.com>,
> 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