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)
Regards
Rüdiger
