Re: svn commit: r1533087 - in /httpd/httpd/trunk/modules/proxy: mod_proxy.h mod_proxy_balancer.c proxy_util.c
This is uglier than creating a subpool... :)
On Oct 17, 2013, at 12:10 PM, Yann Ylavic ylavic@gmail.com wrote:
Maybe using the following macros could help to log worker's (full) name when
no pool is available for ap_proxy_worker_name().
Regards,
Index: modules/proxy/mod_proxy.h
===
--- modules/proxy/mod_proxy.h(revision 1533127)
+++ modules/proxy/mod_proxy.h(working copy)
@@ -328,6 +328,11 @@ do { \
(w)-s-io_buffer_size_set = (c)-io_buffer_size_set;\
} while (0)
+#define PROXY_WORKER_FMT %s%s%s%s
+#define PROXY_WORKER_ARG(w) \
+*(w)-s-uds_path ? unix : , (w)-s-uds_path, \
+*(w)-s-uds_path ?| : , (w)-s-name
+
/* use 2 hashes */
typedef struct {
unsigned int def;
Index: modules/proxy/mod_proxy.c
===
--- modules/proxy/mod_proxy.c(revision 1533127)
+++ modules/proxy/mod_proxy.c(working copy)
@@ -1548,15 +1548,15 @@ static const char *
} else {
reuse = 1;
ap_log_error(APLOG_MARK, APLOG_INFO, 0, cmd-server,
APLOGNO(01145)
- Sharing worker '%s' instead of creating new worker
'%s',
- ap_proxy_worker_name(cmd-pool, worker), new-real);
+ Sharing worker 'PROXY_WORKER_FMT' instead of
creating new worker '%s',
+ PROXY_WORKER_ARG(worker), new-real);
}
for (i = 0; i arr-nelts; i++) {
if (reuse) {
ap_log_error(APLOG_MARK, APLOG_WARNING, 0, cmd-server,
APLOGNO(01146)
- Ignoring parameter '%s=%s' for worker '%s'
because of worker sharing,
- elts[i].key, elts[i].val,
ap_proxy_worker_name(cmd-pool, worker));
+ Ignoring parameter '%s=%s' for worker
'PROXY_WORKER_FMT' because of worker sharing,
+ elts[i].key, elts[i].val,
PROXY_WORKER_ARG(worker));
} else {
const char *err = set_worker_param(cmd-pool, worker,
elts[i].key,
elts[i].val);
@@ -2021,14 +2021,14 @@ static const char *add_member(cmd_parms *cmd, void
if ((err = ap_proxy_define_worker(cmd-pool, worker, balancer,
conf, name, 0)) != NULL)
return apr_pstrcat(cmd-temp_pool, BalancerMember , err, NULL);
ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, cmd-server, APLOGNO(01148)
- Defined worker '%s' for balancer '%s',
- ap_proxy_worker_name(cmd-pool, worker),
balancer-s-name);
+ Defined worker 'PROXY_WORKER_FMT' for balancer '%s',
+ PROXY_WORKER_ARG(worker), balancer-s-name);
PROXY_COPY_CONF_PARAMS(worker, conf);
} else {
reuse = 1;
ap_log_error(APLOG_MARK, APLOG_INFO, 0, cmd-server, APLOGNO(01149)
- Sharing worker '%s' instead of creating new worker
'%s',
- ap_proxy_worker_name(cmd-pool, worker), name);
+ Sharing worker 'PROXY_WORKER_FMT' instead of
creating new worker '%s',
+ PROXY_WORKER_ARG(worker), name);
}
arr = apr_table_elts(params);
@@ -2036,8 +2036,8 @@ static const char *add_member(cmd_parms *cmd, void
for (i = 0; i arr-nelts; i++) {
if (reuse) {
ap_log_error(APLOG_MARK, APLOG_WARNING, 0, cmd-server,
APLOGNO(01150)
- Ignoring parameter '%s=%s' for worker '%s' because
of worker sharing,
- elts[i].key, elts[i].val,
ap_proxy_worker_name(cmd-pool, worker));
+ Ignoring parameter '%s=%s' for worker
'PROXY_WORKER_FMT' because of worker sharing,
+ elts[i].key, elts[i].val, PROXY_WORKER_ARG(worker));
} else {
err = set_worker_param(cmd-pool, worker, elts[i].key,
elts[i].val);
Index: modules/proxy/mod_proxy_balancer.c
===
--- modules/proxy/mod_proxy_balancer.c(revision 1533127)
+++ modules/proxy/mod_proxy_balancer.c(working copy)
@@ -118,8 +118,8 @@ static void init_balancer_members(apr_pool_t *p, s
int worker_is_initialized;
proxy_worker *worker = *workers;
ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(01158)
- Looking at %s - %s initialized?, balancer-s-name,
- ap_proxy_worker_name(p, worker));
+ Looking at PROXY_WORKER_FMT - %s initialized?,
balancer-s-name,
+ PROXY_WORKER_ARG(worker));
worker_is_initialized = PROXY_WORKER_IS_INITIALIZED(worker);
if
Re: svn commit: r1533087 - in /httpd/httpd/trunk/modules/proxy: mod_proxy.h mod_proxy_balancer.c proxy_util.c
On Thu, Oct 17, 2013 at 6:21 PM, Jim Jagielski j...@jagunet.com wrote: This is uglier than creating a subpool... :) Possibly ;) but with the pool being destroyed then... Regards.
Re: svn commit: r1533087 - in /httpd/httpd/trunk/modules/proxy: mod_proxy.h mod_proxy_balancer.c proxy_util.c
On Thu, Oct 17, 2013 at 6:25 PM, Yann Ylavic ylavic@gmail.com wrote:
On Thu, Oct 17, 2013 at 6:21 PM, Jim Jagielski j...@jagunet.com wrote:
This is uglier than creating a subpool... :)
Possibly ;) but with the pool being destroyed then...
Something like :
PROXY_DECLARE(char *) ap_proxy_worker_name(apr_pool_t **p,
proxy_worker *worker)
{
int rv;
apr_uri_t uri;
if (!(*worker-s-uds_path)) {
return worker-s-name;
}
if (!*p) {
/* ugly */
apr_pool_create(p, ap_server_conf-process-pool);
if (!*p) {
/* something is better than nothing :) */
return worker-s-name;
}
}
return apr_pstrcat(*p, unix:, worker-s-uds_path, |,
worker-s-name, NULL);
}
so
that
the caller caller can :
apr_poo_t *p = NULL;
char *s = ap_proxy_worker_name(p, worker);
// play with s
if (p) {
apr_pool_destroy(p);
}
Regards.
Re: svn commit: r1533087 - in /httpd/httpd/trunk/modules/proxy: mod_proxy.h mod_proxy_balancer.c proxy_util.c
That's why I chose what I did... It's only needed once and the pool is around. But yeah, it may be better. On Oct 17, 2013, at 12:25 PM, Yann Ylavic ylavic@gmail.com wrote: On Thu, Oct 17, 2013 at 6:21 PM, Jim Jagielski j...@jagunet.com wrote: This is uglier than creating a subpool... :) Possibly ;) but with the pool being destroyed then... Regards.
Re: svn commit: r1533087 - in /httpd/httpd/trunk/modules/proxy: mod_proxy.h mod_proxy_balancer.c proxy_util.c
On Thu, Oct 17, 2013 at 4:10 PM, j...@apache.org wrote:
Author: jim
Date: Thu Oct 17 14:10:43 2013
New Revision: 1533087
@@ -2087,12 +2089,9 @@ PROXY_DECLARE(int) ap_proxy_acquire_conn
(*conn)-close = 0;
(*conn)-inreslist = 0;
-if (worker-s-uds) {
+if (*worker-s-uds_path) {
if ((*conn)-uds_path == NULL) {
-apr_uri_t puri;
-if (apr_uri_parse(worker-cp-pool, worker-s-name, puri)
== APR_SUCCESS) {
-(*conn)-uds_path = apr_pstrdup(worker-cp-pool,
puri.path);
-}
+(*conn)-uds_path = apr_pstrdup(worker-cp-pool,
worker-s-uds_path);
}
Shouldn't that be either :
(*conn)-uds_path = worker-s-uds_path;
or safer :
(*conn)-uds_path = apr_pstrdup((*conn)-pool,
worker-s-uds_path);
to avoid a leak?
Regards.
Re: svn commit: r1533087 - in /httpd/httpd/trunk/modules/proxy: mod_proxy.h mod_proxy_balancer.c proxy_util.c
For sure the (*conn)-pool is recycled more often...
On Oct 17, 2013, at 3:52 PM, Yann Ylavic ylavic@gmail.com wrote:
On Thu, Oct 17, 2013 at 4:10 PM, j...@apache.org wrote:
Author: jim
Date: Thu Oct 17 14:10:43 2013
New Revision: 1533087
@@ -2087,12 +2089,9 @@ PROXY_DECLARE(int) ap_proxy_acquire_conn
(*conn)-close = 0;
(*conn)-inreslist = 0;
-if (worker-s-uds) {
+if (*worker-s-uds_path) {
if ((*conn)-uds_path == NULL) {
-apr_uri_t puri;
-if (apr_uri_parse(worker-cp-pool, worker-s-name, puri) ==
APR_SUCCESS) {
-(*conn)-uds_path = apr_pstrdup(worker-cp-pool, puri.path);
-}
+(*conn)-uds_path = apr_pstrdup(worker-cp-pool,
worker-s-uds_path);
}
Shouldn't that be either :
(*conn)-uds_path = worker-s-uds_path;
or safer :
(*conn)-uds_path = apr_pstrdup((*conn)-pool,
worker-s-uds_path);
to avoid a leak?
Regards.
