On Sat, Oct 3, 2009 at 3:54 PM, Ruediger Pluem <[email protected]> wrote:
>
> On 03.10.2009 14:54, [email protected] 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).
Index: server/scoreboard.c
===================================================================
--- server/scoreboard.c (revision 821404)
+++ server/scoreboard.c (working copy)
@@ -427,20 +427,17 @@
}
}
-AP_DECLARE(int) ap_update_child_status_from_indexes(int child_num,
- int thread_num,
- int status,
- request_rec *r)
+static int update_child_status_internal(int child_num,
+ int thread_num,
+ int status,
+ conn_rec *c,
+ request_rec *r)
{
int old_status;
worker_score *ws;
process_score *ps;
int mpm_generation;
- if (child_num < 0) {
- return -1;
- }
-
ws = &ap_scoreboard_image->servers[child_num][thread_num];
old_status = ws->status;
ws->status = status;
@@ -468,7 +465,6 @@
ws->conn_bytes = 0;
}
if (r) {
- conn_rec *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);
@@ -477,19 +473,36 @@
sizeof(ws->vhost));
}
}
+ else if (c) {
+ apr_cpystrn(ws->client, ap_get_remote_host(c, NULL,
+ REMOTE_NOLOOKUP, NULL), sizeof(ws->client));
+ }
}
return old_status;
}
+AP_DECLARE(int) ap_update_child_status_from_indexes(int child_num,
+ int thread_num,
+ int status,
+ request_rec *r)
+{
+ if (child_num < 0) {
+ return -1;
+ }
+
+ return update_child_status_internal(child_num, thread_num, status,
+ r->connection, r);
+}
+
AP_DECLARE(int) ap_update_child_status(ap_sb_handle_t *sbh, int status,
request_rec *r)
{
if (!sbh)
return -1;
- return ap_update_child_status_from_indexes(sbh->child_num, sbh->thread_num,
- status, r);
+ return update_child_status_internal(sbh->child_num, sbh->thread_num,
+ status, r->connection, r);
}
AP_DECLARE(int) ap_update_child_status_from_conn(ap_sb_handle_t *sbh, int
status,
@@ -498,11 +511,8 @@
if (!sbh)
return -1;
- request_rec fake_rec;
- fake_rec.connection = c;
-
- return ap_update_child_status_from_indexes(sbh->child_num, sbh->thread_num,
- status, &fake_rec);
+ return update_child_status_internal(sbh->child_num, sbh->thread_num,
+ status, c, NULL);
}
AP_DECLARE(void) ap_time_process_request(ap_sb_handle_t *sbh, int status)
