You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by pg...@apache.org on 2010/08/06 22:07:57 UTC

svn commit: r983116 - /httpd/httpd/trunk/modules/filters/mod_reqtimeout.c

Author: pgollucci
Date: Fri Aug  6 20:07:57 2010
New Revision: 983116

URL: http://svn.apache.org/viewvc?rev=983116&view=rev
Log:
rv is never used which makes the whole if useless

Reported by:	clang static analyzer


Modified:
    httpd/httpd/trunk/modules/filters/mod_reqtimeout.c

Modified: httpd/httpd/trunk/modules/filters/mod_reqtimeout.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/filters/mod_reqtimeout.c?rev=983116&r1=983115&r2=983116&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/filters/mod_reqtimeout.c (original)
+++ httpd/httpd/trunk/modules/filters/mod_reqtimeout.c Fri Aug  6 20:07:57 2010
@@ -190,10 +190,7 @@ static apr_status_t reqtimeout_filter(ap
 #endif
 
             rv = ap_get_brigade(f->next, bb, AP_MODE_GETLINE, APR_NONBLOCK_READ, remaining);
-            if (APR_STATUS_IS_EAGAIN(rv)) {
-                rv = APR_SUCCESS;
-            }
-            else if (rv != APR_SUCCESS) {
+            if (rv != APR_SUCCESS) {
                 break;
             }
 



Re: svn commit: r983116 - /httpd/httpd/trunk/modules/filters/mod_reqtimeout.c

Posted by "Philip M. Gollucci" <pg...@p6m7g8.com>.
On 8/8/2010 2:19 PM, Ruediger Pluem wrote:
>
>
> On 08/06/2010 10:07 PM, pgollucci@apache.org wrote:
>> Author: pgollucci
>> Date: Fri Aug  6 20:07:57 2010
>> New Revision: 983116
>>
>> URL: http://svn.apache.org/viewvc?rev=983116&view=rev
>> Log:
>> rv is never used which makes the whole if useless
>>
>> Reported by:	clang static analyzer
>>
>>
>> Modified:
>>      httpd/httpd/trunk/modules/filters/mod_reqtimeout.c
>>
>> Modified: httpd/httpd/trunk/modules/filters/mod_reqtimeout.c
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/filters/mod_reqtimeout.c?rev=983116&r1=983115&r2=983116&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/modules/filters/mod_reqtimeout.c (original)
>> +++ httpd/httpd/trunk/modules/filters/mod_reqtimeout.c Fri Aug  6 20:07:57 2010
>> @@ -190,10 +190,7 @@ static apr_status_t reqtimeout_filter(ap
>>   #endif
>>
>>               rv = ap_get_brigade(f->next, bb, AP_MODE_GETLINE, APR_NONBLOCK_READ, remaining);
>> -            if (APR_STATUS_IS_EAGAIN(rv)) {
>> -                rv = APR_SUCCESS;
>> -            }
>> -            else if (rv != APR_SUCCESS) {
>> +            if (rv != APR_SUCCESS) {
>>                   break;
>>               }
>
> IMHO this is a change in logic. If ap_get_brigade returns EAGAIN then
> the old code stays in the loop, whereas the new code leaves the loop.
I was thinking about that after I made it.  The relevant tests still 
pass, but I'm not convinced its exercised properly.

I will review this on Tuesday unless you want to do it before then.



-- 
------------------------------------------------------------------------
1024D/DB9B8C1C B90B FBC3 A3A1 C71A 8E70  3F8C 75B8 8FFB DB9B 8C1C
Philip M. Gollucci (pgollucci@p6m7g8.com) c: 703.336.9354
VP Apache Infrastructure; Member, Apache Software Foundation
Committer,                        FreeBSD Foundation
Consultant,                       P6M7G8 Inc.
Sr. System Admin,                 Ridecharge Inc.

Work like you don't need the money,
love like you'll never get hurt,
and dance like nobody's watching.

Re: svn commit: r983116 - /httpd/httpd/trunk/modules/filters/mod_reqtimeout.c

Posted by Ruediger Pluem <rp...@apache.org>.

On 08/06/2010 10:07 PM, pgollucci@apache.org wrote:
> Author: pgollucci
> Date: Fri Aug  6 20:07:57 2010
> New Revision: 983116
> 
> URL: http://svn.apache.org/viewvc?rev=983116&view=rev
> Log:
> rv is never used which makes the whole if useless
> 
> Reported by:	clang static analyzer
> 
> 
> Modified:
>     httpd/httpd/trunk/modules/filters/mod_reqtimeout.c
> 
> Modified: httpd/httpd/trunk/modules/filters/mod_reqtimeout.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/filters/mod_reqtimeout.c?rev=983116&r1=983115&r2=983116&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/filters/mod_reqtimeout.c (original)
> +++ httpd/httpd/trunk/modules/filters/mod_reqtimeout.c Fri Aug  6 20:07:57 2010
> @@ -190,10 +190,7 @@ static apr_status_t reqtimeout_filter(ap
>  #endif
>  
>              rv = ap_get_brigade(f->next, bb, AP_MODE_GETLINE, APR_NONBLOCK_READ, remaining);
> -            if (APR_STATUS_IS_EAGAIN(rv)) {
> -                rv = APR_SUCCESS;
> -            }
> -            else if (rv != APR_SUCCESS) {
> +            if (rv != APR_SUCCESS) {
>                  break;
>              }

IMHO this is a change in logic. If ap_get_brigade returns EAGAIN then
the old code stays in the loop, whereas the new code leaves the loop.

Regards

RĂ¼diger