2017-03-27 19:09 GMT+02:00 Luca Toscano <toscano.l...@gmail.com>: > Hi Yann, > > 2017-03-27 8:56 GMT+02:00 Yann Ylavic <ylavic....@gmail.com>: > >> Hi Luca, >> >> On Mon, Mar 20, 2017 at 1:25 PM, Luca Toscano <toscano.l...@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
