On Fri, Jul 20, 2018 at 2:54 PM, Ruediger Pluem <rpl...@apache.org> wrote:
>
> On 07/20/2018 02:38 PM, Yann Ylavic wrote:
>> On Fri, Jul 20, 2018 at 12:08 PM, Ruediger Pluem <rpl...@apache.org> wrote:
>>>
>>> On 07/19/2018 12:36 AM, yla...@apache.org wrote:
>>>>
>>>> - if (!c->data_in_input_filters || ap_run_input_pending(c) != OK) {
>>>> + if (ap_run_input_pending(c) != OK) {
>>>
>>> We have a different code flow here now. If c->data_in_input_filters is 0,
>>> then
>>> ap_filter_input_pending does iterate over the ring whereas it did not do
>>> before,
>>> because it was not called.
>>
>> Right, though ap_filter_input_pending() iteration is quite cheap, and
>> could be even cheaper if pending input and output filters were handled
>> separetely (next step...).
>>
>> I prefer to keep the logic in ap_filter_*_pending() if you don't mind,
>> and avoid global c->data_in_*_filter checks all over the place
>> (possibly they'll disappear from conn_rec some day).
>> For now c->data_in_*_filter are used internally (core) to positively
>> force pending data, never negatively (and that's wise I think).
>
> So if c->data_in_input_filters is 0 the iteration is not expected to return
> OK,
> as otherwise we would have an inconsistency between c->data_in_input_filters
> and the ring, correct?
Well, it's hard/unwise to keep a per-connection flag aligned with a
per-filter feature, that's why those flags should disappear I think.
But yes, when ap_run_input_pending() is called from
ap_process_request() it will never return OK if
c->data_in_input_filters is 0.
The issue, I think, is rather that if c->data_in_*_filters is 1 it
will not change until the next call to ap_process_[async_]request(),
even though the filter chain may be emptied until then, so
ap_filter_*_pending() may return OK while it shouldn't.
Hmm, let me look at this more closely, it seems that these flags
should really disappear now.
> On the other hand this inconsistency is possible if ap_run_input_pending is
> called from
> other locations in the code than the above where we did not have this
> !c->data_in_input_filters check, correct?
This says the same thing as I said above right?
Regards,
Yann.
