httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ruediger Pluem <rpl...@apache.org>
Subject Re: svn commit: r1776575 - in /httpd/httpd/trunk: docs/log-message-tags/next-number docs/manual/mod/mod_remoteip.xml modules/metadata/mod_remoteip.c
Date Mon, 30 Jan 2017 09:55:43 GMT


On 01/28/2017 06:59 PM, Daniel Ruggeri wrote:
> Ruediger Pluem wrote:
>> +    /* try to read a header's worth of data */
>> +    while (!ctx->done) {
>> +        if (APR_BRIGADE_EMPTY(ctx->bb)) {
>> +            ret = ap_get_brigade(f->next, ctx->bb, ctx->mode, block,
>> +                                 ctx->need - ctx->rcvd);
>> +            if (ret != APR_SUCCESS) {
>> +                return ret;
>> +            }
>> +        }
>> +        if (APR_BRIGADE_EMPTY(ctx->bb)) {
> What about the case of an non blocking read where the upstream filter
> returns an empty brigade
> and APR_SUCCESS. This is equal to returning EAGAIN.
> 
>> +            return APR_EOF;
>> +        }
> 
> Coming back to this one after correcting the setaside stuff... Is this
> what you have in mind or should we actually return APR_EAGAIN?
> 
> return block == APR_NONBLOCK_READ ? APR_SUCCESS : APR_EOF;
> 

IMHO you will hit the APR_BRIGADE_EMPTY(ctx->bb) with APR_SUCCESS returned case only
in the non blocking case. So it would be sufficient to replace

return APR_EOF

with

return APR_SUCCESS.

You proposal is a little bit more fail safe and hence probably a good idea.
Furthermore I think you could move the second check for an empty brigade inside
the block checking it firstly (at the end).

Regards

RĂ¼diger

Mime
View raw message