httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stefan Fritsch ...@sfritsch.de>
Subject Re: svn commit: r921378 - in /httpd/httpd/trunk: CHANGES modules/filters/mod_reqtimeout.c
Date Mon, 15 Mar 2010 20:28:46 GMT
On Mon, 15 Mar 2010, Ruediger Pluem wrote:
>>>> @@ -114,6 +105,24 @@ static apr_status_t reqtimeout_filter(ap
>>>>          return ap_get_brigade(f->next, bb, mode, block, readbytes);
>>>>      }
>>>>
>>>> +    if (!ccfg->socket) {
>>>> +        core_net_rec *net_rec;
>>>> +        ap_filter_t *core_in = f->next;
>>>> +
>>>> +        while (core_in && core_in->frec != ap_core_input_filter_handle)
>>>> +            core_in = core_in->next;
>>>> +
>>>> +        if (!core_in) {
>>>> +            ap_log_cerror(APLOG_MARK, APLOG_WARNING, 0, f->c,
>>>> +                          "mod_reqtimeout: Can't get socket "
>>>> +                          "handle from core_input_filter");
>>>> +            ap_remove_input_filter(f);
>>>> +            return ap_get_brigade(f->next, bb, mode, block, readbytes);
>>>> +        }
>>>> +        net_rec = core_in->ctx;
>>>> +        ccfg->socket = net_rec->client_socket;
>>>> +    }
>>>> +
>>>
>>> Hm, this looks kind of ugly. Why not leaving things in
>>> r->connection->conn_config and run through both hooks (pre_connection and
>>> process_connection). The first one stores the socket, the second one
>>> applies
>>> the filter if ever reached.
>>
>> I wanted to avoid allocating the memory for mod_reqtimeout's conn_config
>> if it is not enabled anyway. Without the conn_config, there is no place
>> to store the socket. If you think wasting a bit of memory is better than
>> that loop, I am fine with that too. But usually there are only very few
>
> I think wasting this small amount of memory is ok and if someone does not
> want mod_reqtimeout he should simply not load it and in this case there
> is no waste at all.

Found a simpler way:

     if (!ccfg->socket) {
         ccfg->socket = ap_get_module_config(f->c->conn_config, &core_module);
     }

Is that OK or are other modules not supposed to use core_module?

Cheers,
Stefan

Mime
View raw message