On 9/10/20 9:31 AM, Stefan Eissing wrote:
>
>
>> Am 10.09.2020 um 09:29 schrieb Ruediger Pluem <rpl...@apache.org>:
>>
>>
>>
>> On 9/9/20 10:21 AM, Stefan Eissing wrote:
>>>
>>>
>>>> Am 08.09.2020 um 21:11 schrieb Ruediger Pluem <rpl...@apache.org>:
>>>>
>>>>
>>>>
>>>> On 9/8/20 9:22 AM, Stefan Eissing wrote:
>>>>>
>>>>>
>>>>>> Am 08.09.2020 um 08:27 schrieb Ruediger Pluem <rpl...@apache.org>:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 8/21/20 9:20 AM, Ruediger Pluem wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 8/20/20 11:38 AM, Stefan Eissing wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>> Am 20.08.2020 um 11:35 schrieb Ruediger Pluem <rpl...@apache.org>:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 8/20/20 10:47 AM, Stefan Eissing wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>> Am 20.08.2020 um 10:01 schrieb Ruediger Pluem <rpl...@apache.org>:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On 8/19/20 12:18 PM, Stefan Eissing wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>> Am 19.08.2020 um 12:08 schrieb Ruediger Pluem <rpl...@apache.org>:
>>
>>>>>> Any feedback or comments?
>>>>>
>>>>> Sorry about the delay, my inbox in unhealthy these days.
>>>>
>>>> No problem. Even more thanks then for taking time for a review.
>>>
>>> Thanks for improving this.
>>>>
>>>>>
>>>>> Had a quick look. My read: it looks like a good approach. The request
>>>>> still needs to be processed in a worker, but that should be very light
>>>>> and fast. I was first confused by the "early_http_status" term as there
>>>>> is the "103 early hints" intermediate response code stuck in my brain.
>>>>> Maybe we should just call it http_status and have a
>>>>> HTTP_NEEDS_FURTHER_PROCESSING (0) in our server.
>>>>
>>>> Updated the PR and renamed early_http_status to http_status.
>>>> What do you mean with / what is the purpose of
>>>> HTTP_NEEDS_FURTHER_PROCESSING? Should http_status be initialized with this
>>>> define
>>>> or do you want to replace conditions of the type (http_status) with
>>>> (http_status != HTTP_NEEDS_FURTHER_PROCESSING)?
>>>> At what place should we define it? In h2.h?
>>>
>>> Just a thought that 0 could indicate that the http status has not been
>>> determined yet (default) or in case of an early error the code to return.
>>> Which then prevents further processing. The name for such a value was not
>>> entirely serious. We could just check on != 0.
>> It is already the case that a value of 0 in http_status indicates that the
>> http status has not been determined yet and 0 is
>> already the default value via the apr_pcalloc of the structure.
>>
>> Would you like to see the following?
>>
>> 1. Make a define like HTTP_NEEDS_FURTHER_PROCESSING (I would propose
>> H2_HTTP_STATUS_UNSET, sweet naming discussion :-))
>> 2. In addition to the apr_pcalloc which already makes http_status zero set
>> http_status explicitly to H2_HTTP_STATUS_UNSET.
>> 3. Replace the (http_status) conditions in the ifs with (http_status !=
>> H2_HTTP_STATUS_UNSET)
>
> I like that very much. I think it makes good reading.
That was a quick reply. So the now updated PR is fine for you?
Regards
Rüdiger
