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: r821307 - in /httpd/httpd/trunk: include/scoreboard.h modules/experimental/mod_noloris.c server/scoreboard.c
Date Sat, 03 Oct 2009 20:21:16 GMT


On 10/03/2009 10:12 PM, Jeff Trawick wrote:
> On Sat, Oct 3, 2009 at 3:54 PM, Ruediger Pluem <rpluem@apache.org> wrote:
> 
>> On 03.10.2009 14:54, jim@apache.org wrote:
>>> Author: jim
>>> Date: Sat Oct  3 12:54:35 2009
>>> New Revision: 821307
>>>
>>> URL: http://svn.apache.org/viewvc?rev=821307&view=rev
>>> Log:
>>> Provide new  ap_update_child_status_from_conn() mostly
>>> for use with mod_noloris.c Add some logic protection, for
>>> NULL ref, which shoulda be there in any case.
>>>
>>> Modified:
>>>     httpd/httpd/trunk/include/scoreboard.h
>>>     httpd/httpd/trunk/modules/experimental/mod_noloris.c
>>>     httpd/httpd/trunk/server/scoreboard.c
>>>
>>
>>> Modified: httpd/httpd/trunk/server/scoreboard.c
>>> URL:
>> http://svn.apache.org/viewvc/httpd/httpd/trunk/server/scoreboard.c?rev=821307&r1=821306&r2=821307&view=diff
>> ==============================================================================
>>> --- httpd/httpd/trunk/server/scoreboard.c (original)
>>> +++ httpd/httpd/trunk/server/scoreboard.c Sat Oct  3 12:54:35 2009
>>> @@ -490,6 +492,19 @@
>>>                                                 status, r);
>>>  }
>>>
>>> +AP_DECLARE(int) ap_update_child_status_from_conn(ap_sb_handle_t *sbh,
>> int status,
>>> +                                       conn_rec *c)
>>> +{
>>> +    if (!sbh)
>>> +        return -1;
>>> +
>>> +    request_rec fake_rec;
>>> +    fake_rec.connection = c;
>> Shouldn't we set fake_rec.per_dir_config and fake_rec.server to NULL to
>> play safe?
>>
> 
> I think you're right, but it is still hard to follow.  I think a bit of
> refactoring is in order (attached, untexted).

+1. Looks better and cleaner. Maybe

@@ -468,7 +465,6 @@
             ws->conn_bytes = 0;
         }
         if (r) {
-            conn_rec *c = r->connection;

+            if (!c) {
+                c = r->connection;
+            }
             apr_cpystrn(ws->client, ap_get_remote_host(c, r->per_dir_config,
                         REMOTE_NOLOOKUP, NULL), sizeof(ws->client));
             copy_request(ws->request, sizeof(ws->request), r);

to avoid someone calling it with setting r but forgetting to set c.
But maybe I am just too cautious here :-).

Regards

RĂ¼diger




Mime
View raw message