You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Igor Galić <i....@brainsware.org> on 2011/11/23 13:07:23 UTC

Re: svn commit: r1205274 - /httpd/httpd/trunk/modules/mappers/mod_negotiation.c


[snip]
> mod_negotiation: don't return non-AP_FILTER_ERROR filter return
> values
[snip]
> +        rv = ap_pass_brigade(r->output_filters, bb);
> +        if (rv != APR_SUCCESS) {
> +            if (rv != AP_FILTER_ERROR) {
> +                ap_log_rerror(APLOG_MARK, APLOG_DEBUG, rv, r,
> +                              "ap_pass_brigade returned %d", rv);
> +                return HTTP_INTERNAL_SERVER_ERROR;
> +            }
> +            return AP_FILTER_ERROR;
> +        }
> +        return OK;

It looks like exactly this kind of code would or should be used
all over the place -- maybe we should put that in a function/macro?

[snip]

-- 
Igor Galić

Tel: +43 (0) 664 886 22 883
Mail: i.galic@brainsware.org
URL: http://brainsware.org/
GPG: 6880 4155 74BD FD7C B515  2EA5 4B1D 9E08 A097 C9AE


Re: svn commit: r1205274 - /httpd/httpd/trunk/modules/mappers/mod_negotiation.c

Posted by Jim Jagielski <ji...@jaguNET.com>.
Add in r1205419 and used in r1205423
On Nov 23, 2011, at 9:29 AM, Eric Covener wrote:

> On Wed, Nov 23, 2011 at 9:15 AM, Jim Jagielski <ji...@apache.org> wrote:
>> 
>> On Nov 23, 2011, at 7:07 AM, Igor Galić wrote:
>> 
>>> 
>>> 
>>> [snip]
>>>> mod_negotiation: don't return non-AP_FILTER_ERROR filter return
>>>> values
>>> [snip]
>>>> +        rv = ap_pass_brigade(r->output_filters, bb);
>>>> +        if (rv != APR_SUCCESS) {
>>>> +            if (rv != AP_FILTER_ERROR) {
>>>> +                ap_log_rerror(APLOG_MARK, APLOG_DEBUG, rv, r,
>>>> +                              "ap_pass_brigade returned %d", rv);
>>>> +                return HTTP_INTERNAL_SERVER_ERROR;
>>>> +            }
>>>> +            return AP_FILTER_ERROR;
>>>> +        }
>>>> +        return OK;
>>> 
>>> It looks like exactly this kind of code would or should be used
>>> all over the place -- maybe we should put that in a function/macro?
>>> 
>> 
>> ap_pass_brigade_errchk(r, bb)
>> 
>> and the above could compress to
>> 
>>        return ap_pass_brigade_errchk(r, bb);
>> 
>> +1
>> 
>> Unless someone beats me to it, will add to trunk and backport
>> to 2.4...
> 
> maybe additionally a char* for the caller to add a msg -- it was
> useful in this case to find out what the handler was.
> 


Re: svn commit: r1205274 - /httpd/httpd/trunk/modules/mappers/mod_negotiation.c

Posted by Eric Covener <co...@gmail.com>.
On Wed, Nov 23, 2011 at 9:15 AM, Jim Jagielski <ji...@apache.org> wrote:
>
> On Nov 23, 2011, at 7:07 AM, Igor Galić wrote:
>
>>
>>
>> [snip]
>>> mod_negotiation: don't return non-AP_FILTER_ERROR filter return
>>> values
>> [snip]
>>> +        rv = ap_pass_brigade(r->output_filters, bb);
>>> +        if (rv != APR_SUCCESS) {
>>> +            if (rv != AP_FILTER_ERROR) {
>>> +                ap_log_rerror(APLOG_MARK, APLOG_DEBUG, rv, r,
>>> +                              "ap_pass_brigade returned %d", rv);
>>> +                return HTTP_INTERNAL_SERVER_ERROR;
>>> +            }
>>> +            return AP_FILTER_ERROR;
>>> +        }
>>> +        return OK;
>>
>> It looks like exactly this kind of code would or should be used
>> all over the place -- maybe we should put that in a function/macro?
>>
>
> ap_pass_brigade_errchk(r, bb)
>
> and the above could compress to
>
>        return ap_pass_brigade_errchk(r, bb);
>
> +1
>
> Unless someone beats me to it, will add to trunk and backport
> to 2.4...

maybe additionally a char* for the caller to add a msg -- it was
useful in this case to find out what the handler was.

Re: svn commit: r1205274 - /httpd/httpd/trunk/modules/mappers/mod_negotiation.c

Posted by Jim Jagielski <ji...@apache.org>.
On Nov 23, 2011, at 7:07 AM, Igor Galić wrote:

> 
> 
> [snip]
>> mod_negotiation: don't return non-AP_FILTER_ERROR filter return
>> values
> [snip]
>> +        rv = ap_pass_brigade(r->output_filters, bb);
>> +        if (rv != APR_SUCCESS) {
>> +            if (rv != AP_FILTER_ERROR) {
>> +                ap_log_rerror(APLOG_MARK, APLOG_DEBUG, rv, r,
>> +                              "ap_pass_brigade returned %d", rv);
>> +                return HTTP_INTERNAL_SERVER_ERROR;
>> +            }
>> +            return AP_FILTER_ERROR;
>> +        }
>> +        return OK;
> 
> It looks like exactly this kind of code would or should be used
> all over the place -- maybe we should put that in a function/macro?
> 

ap_pass_brigade_errchk(r, bb)

and the above could compress to

	return ap_pass_brigade_errchk(r, bb);

+1

Unless someone beats me to it, will add to trunk and backport
to 2.4...


Re: svn commit: r1205274 - /httpd/httpd/trunk/modules/mappers/mod_negotiation.c

Posted by Jim Jagielski <ji...@apache.org>.
On Nov 23, 2011, at 7:07 AM, Igor Galić wrote:

> 
> 
> [snip]
>> mod_negotiation: don't return non-AP_FILTER_ERROR filter return
>> values
> [snip]
>> +        rv = ap_pass_brigade(r->output_filters, bb);
>> +        if (rv != APR_SUCCESS) {
>> +            if (rv != AP_FILTER_ERROR) {
>> +                ap_log_rerror(APLOG_MARK, APLOG_DEBUG, rv, r,
>> +                              "ap_pass_brigade returned %d", rv);
>> +                return HTTP_INTERNAL_SERVER_ERROR;
>> +            }
>> +            return AP_FILTER_ERROR;
>> +        }
>> +        return OK;
> 
> It looks like exactly this kind of code would or should be used
> all over the place -- maybe we should put that in a function/macro?
> 

ap_pass_brigade_errchk(r, bb)

and the above could compress to

	return ap_pass_brigade_errchk(r, bb);

+1

Unless someone beats me to it, will add to trunk and backport
to 2.4...