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
