httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jim Jagielski <...@jaguNET.com>
Subject Re: svn commit: r503863 - /httpd/httpd/trunk/CHANGES
Date Tue, 06 Feb 2007 20:25:59 GMT

On Feb 6, 2007, at 3:07 PM, Ruediger Pluem wrote:

>
>
> On 02/06/2007 06:26 PM, Jim Jagielski wrote:
>> I don't know what's happening with my emails, but they
>> appear to be getting dropped left and right.
>>
>> I had responded to Joe's email, saying that I must be
>> misunderstanding his concern, but I haven't seen that
>> make it's way through yet.
>>
>> If I'm understanding it correctly, what we should be
>> doing is honoring the EAGAIN (if any) at line 295 and
>> returning that (w/o the state change). But it seems
>> that Joe doesn't think that'll work... I'm confused.
>> The previous patch wasn't correct, that's for sure.
>>
>
>> From my understanding the following needs to be done:
>
> Index: http_filters.c
> ===================================================================
> --- http_filters.c      (Revision 504183)
> +++ http_filters.c      (Arbeitskopie)
> @@ -294,6 +294,12 @@
>                  if (ctx->state == BODY_CHUNK) {
>                      rv = ap_get_brigade(f->next, bb, AP_MODE_GETLINE,
>                                          block, 0);
> +                    if (block == APR_NONBLOCK_READ &&
> +                        (((rv == APR_SUCCESS) && (APR_BRIGADE_EMPTY 
> (bb))) ||
> +                         (APR_STATUS_IS_EAGAIN(rv)))) {
> +                        apr_brigade_cleanup(bb);
> +                        return APR_EAGAIN;
> +                    }
>                      apr_brigade_cleanup(bb);
>                  } else {
>                      rv = APR_SUCCESS;
>
>
> This is slightly different than your patch Jim, as it also checks
> for an empty brigade with a APR_SUCESSS return code as we do
> a few lines later. Put possibly this could be also wrong here if the
> *successfull* read of an empty line in AP_MODE_GET_LINE returns an
> empty brigade.  In this case the patch you sent to Joe and me  
> should be correct:
>

That was my concern... And since we always do the cleanup,
duplicating that call doesn't seem to make sense... ;)

(here's hoping this email makes it through :) )


> Index: modules/http/http_filters.c
> ===================================================================
> --- modules/http/http_filters.c (revision 504222)
> +++ modules/http/http_filters.c (working copy)
> @@ -295,6 +295,10 @@
>                      rv = ap_get_brigade(f->next, bb, AP_MODE_GETLINE,
>                                          block, 0);
>                      apr_brigade_cleanup(bb);
> +                    if (block == APR_NONBLOCK_READ &&
> +                        (APR_STATUS_IS_EAGAIN(rv))) {
> +                        return APR_EAGAIN;
> +                    }
>                  } else {
>                      rv = APR_SUCCESS;
>                  }
>
>
>
>
> Regards
>
> RĂ¼diger
>


Mime
View raw message