Re: [users@httpd] Problem when using nested if statements in apache 2.4

[Luca Toscano <to...@gmail.com>]

2017-03-27 19:09 GMT+02:00 Luca Toscano <to...@gmail.com>:

> Hi Yann,
>
> 2017-03-27 8:56 GMT+02:00 Yann Ylavic <yl...@gmail.com>:
>
>> Hi Luca,
>>
>> On Mon, Mar 20, 2017 at 1:25 PM, Luca Toscano <to...@gmail.com>
>> wrote:
>> >
>> > Documentation updated with the current status, plus the following patch
>> > seems to allow nested if blocks (probably not the best one but it is a
>> pof):
>> >
>> > http://home.apache.org/~elukey/httpd-trunk-core-nested_if_blocks.patch
>>
>> LGTM (nit: ap_if_walk_sub() needs not be AP_DECLARE()d since it's a
>> local helper only).
>>
>
> Thanks a lot for the feedback, I modified the patch to include your
> suggestion and another one from Jim (checking whether or not the
> ap_if_walk_sub calls return something different than OK, hope that the
> implementation is what was expected). Ran also the test suite in trunk, no
> failure registered (Jacob: make check is really awesome).
>
> I'd like to perform some performance tests before proceeding, this code
> adds a (probably negligible in 90% of the use case) overhead to each
> ap_if_walk call (running twice for each request afaics).
>
> Any other comment/review will be really appreciated :)
>
>
Updates:

- the original patch evolved up to http://home.apache.org/~
elukey/httpd-trunk-core-nested_if_blocks.patch but it was a bit too hacky.

- Jacob reviewed the code and suggested to investigate the use of
now_merged to clean up the code, and a new patch came to life:
http://home.apache.org/~elukey/httpd-trunk-core-nested_if_blocks_v2.patch

- Added some tests to the suite in http://home.apache.org/~
elukey/httpd-framework-nested-ifs.patch, everything seems to work fine. My
understanding is that the tests are leveraging the "Header merge" feature
to simulate the order of merging with different <If> condition evaluations.
I added the same block of nested <If>/ElseIf/Else to
Directory/Location/Files and the result looks consistent.

Comments welcome, I think the work is almost done but I might have missed
something..

Luca

Re: [users@httpd] Problem when using nested if statements in apache 2.4

[Luca Toscano <to...@gmail.com>]

2017-04-19 10:05 GMT+02:00 Luca Toscano <to...@gmail.com>:

>
>
> 2017-03-27 19:09 GMT+02:00 Luca Toscano <to...@gmail.com>:
>
>> Hi Yann,
>>
>> 2017-03-27 8:56 GMT+02:00 Yann Ylavic <yl...@gmail.com>:
>>
>>> Hi Luca,
>>>
>>> On Mon, Mar 20, 2017 at 1:25 PM, Luca Toscano <to...@gmail.com>
>>> wrote:
>>> >
>>> > Documentation updated with the current status, plus the following patch
>>> > seems to allow nested if blocks (probably not the best one but it is a
>>> pof):
>>> >
>>> > http://home.apache.org/~elukey/httpd-trunk-core-nested_if_blocks.patch
>>>
>>> LGTM (nit: ap_if_walk_sub() needs not be AP_DECLARE()d since it's a
>>> local helper only).
>>>
>>
>> Thanks a lot for the feedback, I modified the patch to include your
>> suggestion and another one from Jim (checking whether or not the
>> ap_if_walk_sub calls return something different than OK, hope that the
>> implementation is what was expected). Ran also the test suite in trunk, no
>> failure registered (Jacob: make check is really awesome).
>>
>> I'd like to perform some performance tests before proceeding, this code
>> adds a (probably negligible in 90% of the use case) overhead to each
>> ap_if_walk call (running twice for each request afaics).
>>
>> Any other comment/review will be really appreciated :)
>>
>>
> Updates:
>
> - the original patch evolved up to http://home.apache.org/~elukey
> /httpd-trunk-core-nested_if_blocks.patch but it was a bit too hacky.
>
> - Jacob reviewed the code and suggested to investigate the use of
> now_merged to clean up the code, and a new patch came to life:
> http://home.apache.org/~elukey/httpd-trunk-core-nested_if_blocks_v2.patch
>
> - Added some tests to the suite in http://home.apache.org/~elukey
> /httpd-framework-nested-ifs.patch, everything seems to work fine. My
> understanding is that the tests are leveraging the "Header merge" feature
> to simulate the order of merging with different <If> condition evaluations.
> I added the same block of nested <If>/ElseIf/Else to
> Directory/Location/Files and the result looks consistent.
>
> Comments welcome, I think the work is almost done but I might have missed
> something..
>


For the records the trunk patch has been committed in r1792589 and the
related tests in r1793187.

Luca