summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorChris Darroch <chrisd@apache.org>2007-01-02 19:26:00 +0100
committerChris Darroch <chrisd@apache.org>2007-01-02 19:26:00 +0100
commitbf27d1a35452d1833cabf40f151e1a04eb583226 (patch)
treeafa5a94db7279181917f9e59171ee8c3803da415
parentRename functions and variables for consistency, and move some functions (diff)
downloadapache2-bf27d1a35452d1833cabf40f151e1a04eb583226.tar.xz
apache2-bf27d1a35452d1833cabf40f151e1a04eb583226.zip
Handle error conditions in dbd_construct() properly. Simplify
ap_dbd_open() and use correct arguments to apr_dbd_error() when non-threaded. Register correct cleanup data in non-threaded ap_dbd_acquire() and ap_dbd_cacquire(). Clean up configuration data and merge function. Use ap_log_error() wherever possible. git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@491884 13f79535-47bb-0310-9956-ffa450edef68
-rw-r--r--CHANGES7
-rw-r--r--modules/database/mod_dbd.c224
2 files changed, 118 insertions, 113 deletions
diff --git a/CHANGES b/CHANGES
index c561ace1a0..5fa4739b31 100644
--- a/CHANGES
+++ b/CHANGES
@@ -2,6 +2,13 @@
Changes with Apache 2.3.0
[Remove entries to the current 2.0 and 2.2 section below, when backported]
+ *) mod_dbd: Handle error conditions in dbd_construct() properly.
+ Simplify ap_dbd_open() and use correct arguments to apr_dbd_error()
+ when non-threaded. Register correct cleanup data in non-threaded
+ ap_dbd_acquire() and ap_dbd_cacquire(). Clean up configuration data
+ and merge function. Use ap_log_error() wherever possible.
+ [Chris Darroch, Nick Kew]
+
*) mod_proxy_http: Handle request bodies larger than 2 GB by converting
the Content-Length header of the request correctly. PR 40883.
[Ruediger Pluem, toadie <toadie643 gmail.com>]
diff --git a/modules/database/mod_dbd.c b/modules/database/mod_dbd.c
index bbe5437f43..56270f9153 100644
--- a/modules/database/mod_dbd.c
+++ b/modules/database/mod_dbd.c
@@ -20,15 +20,20 @@
* http://apache.webthing.com/database/
*/
-#include <ctype.h>
+#include "apr_reslist.h"
+#include "apr_strings.h"
+#include "apr_hash.h"
+#include "apr_lib.h"
+#include "apr_dbd.h"
+
+#define APR_WANT_MEMFUNC
+#define APR_WANT_STRFUNC
+#include "apr_want.h"
#include "http_protocol.h"
#include "http_config.h"
#include "http_log.h"
#include "http_request.h"
-#include "apr_reslist.h"
-#include "apr_strings.h"
-#include "apr_dbd.h"
#include "mod_dbd.h"
extern module AP_MODULE_DECLARE_DATA dbd_module;
@@ -49,6 +54,7 @@ struct dbd_prepared {
};
typedef struct {
+ server_rec *server;
const char *name;
const char *params;
int persist;
@@ -61,10 +67,10 @@ typedef struct {
int nkeep;
int nmax;
int exptime;
+ int set;
#else
ap_dbd_t *rec;
#endif
- unsigned int set;
} svr_cfg;
typedef enum { cmd_name, cmd_params, cmd_persist,
@@ -88,6 +94,7 @@ static void *create_dbd_config(apr_pool_t *pool, server_rec *s)
{
svr_cfg *svr = apr_pcalloc(pool, sizeof(svr_cfg));
+ svr->server = s;
svr->name = no_dbdriver; /* to generate meaningful error messages */
svr->params = ""; /* don't risk segfault on misconfiguration */
svr->persist = -1;
@@ -107,24 +114,23 @@ static void *merge_dbd_config(apr_pool_t *pool, void *basev, void *addv)
svr_cfg *add = (svr_cfg*) addv;
svr_cfg *svr = apr_pcalloc(pool, sizeof(svr_cfg));
+ svr->server = add->server;
svr->name = (add->name != no_dbdriver) ? add->name : base->name;
svr->params = strcmp(add->params, "") ? add->params : base->params;
- svr->persist = (add->persist == -1) ? base->persist : add->persist;
+ svr->persist = (add->persist != -1) ? add->persist : base->persist;
#if APR_HAS_THREADS
svr->nmin = (add->set&NMIN_SET) ? add->nmin : base->nmin;
svr->nkeep = (add->set&NKEEP_SET) ? add->nkeep : base->nkeep;
svr->nmax = (add->set&NMAX_SET) ? add->nmax : base->nmax;
svr->exptime = (add->set&EXPTIME_SET) ? add->exptime : base->exptime;
#endif
- svr->set = add->set | base->set;
- svr->prepared = (add->prepared != NULL) ? add->prepared : base->prepared;
return svr;
}
#define ISINT(val) \
for (p = val; *p; ++p) \
- if (!isdigit(*p)) \
+ if (!apr_isdigit(*p)) \
return "Argument must be numeric!"
static const char *dbd_param(cmd_parms *cmd, void *dconf, const char *val)
@@ -258,7 +264,7 @@ static int dbd_post_config(apr_pool_t *pconf, apr_pool_t *plog,
for (sp = s; sp; sp = sp->next) {
svr_cfg *svr = ap_get_module_config(sp->module_config, &dbd_module);
- const char *key = apr_psprintf(s->process->pool, "%pp", s);
+ const char *key = apr_psprintf(ptemp, "%pp", s);
svr->prepared = apr_hash_get(dbd_prepared_defns, key,
APR_HASH_KEY_STRING);
@@ -321,8 +327,9 @@ static apr_status_t dbd_construct(void **data_ptr,
/* this pool is mostly so dbd_close can destroy the prepared stmts */
rv = apr_pool_create(&rec->pool, pool);
if (rv != APR_SUCCESS) {
- ap_log_perror(APLOG_MARK, APLOG_CRIT, rv, pool,
- "DBD: Failed to create memory pool");
+ ap_log_error(APLOG_MARK, APLOG_CRIT, rv, svr->server,
+ "DBD: Failed to create memory pool");
+ return rv;
}
/* The driver is loaded at config time now, so this just checks a hash.
@@ -334,24 +341,25 @@ static apr_status_t dbd_construct(void **data_ptr,
if (rv != APR_SUCCESS) {
switch (rv) {
case APR_ENOTIMPL:
- ap_log_perror(APLOG_MARK, APLOG_ERR, rv, rec->pool,
- "DBD: driver for %s not available", svr->name);
+ ap_log_error(APLOG_MARK, APLOG_ERR, rv, svr->server,
+ "DBD: driver for %s not available", svr->name);
break;
case APR_EDSOOPEN:
- ap_log_perror(APLOG_MARK, APLOG_ERR, rv, rec->pool,
- "DBD: can't find driver for %s", svr->name);
+ ap_log_error(APLOG_MARK, APLOG_ERR, rv, svr->server,
+ "DBD: can't find driver for %s", svr->name);
break;
case APR_ESYMNOTFOUND:
- ap_log_perror(APLOG_MARK, APLOG_ERR, rv, rec->pool,
- "DBD: driver for %s is invalid or corrupted",
- svr->name);
+ ap_log_error(APLOG_MARK, APLOG_ERR, rv, svr->server,
+ "DBD: driver for %s is invalid or corrupted",
+ svr->name);
break;
default:
- ap_log_perror(APLOG_MARK, APLOG_ERR, rv, rec->pool,
- "DBD: mod_dbd not compatible with APR in get_driver");
+ ap_log_error(APLOG_MARK, APLOG_ERR, rv, svr->server,
+ "DBD: mod_dbd not compatible with APR in get_driver");
break;
}
+ apr_pool_destroy(rec->pool);
return rv;
}
@@ -359,33 +367,39 @@ static apr_status_t dbd_construct(void **data_ptr,
if (rv != APR_SUCCESS) {
switch (rv) {
case APR_EGENERAL:
- ap_log_perror(APLOG_MARK, APLOG_ERR, rv, rec->pool,
- "DBD: Can't connect to %s", svr->name);
+ ap_log_error(APLOG_MARK, APLOG_ERR, rv, svr->server,
+ "DBD: Can't connect to %s", svr->name);
break;
default:
- ap_log_perror(APLOG_MARK, APLOG_ERR, rv, rec->pool,
- "DBD: mod_dbd not compatible with APR in open");
+ ap_log_error(APLOG_MARK, APLOG_ERR, rv, svr->server,
+ "DBD: mod_dbd not compatible with APR in open");
break;
}
+ apr_pool_destroy(rec->pool);
return rv;
}
- *data_ptr = rec;
-
rv = dbd_prepared_init(rec->pool, svr, rec);
if (rv != APR_SUCCESS) {
const char *errmsg = apr_dbd_error(rec->driver, rec->handle, rv);
- ap_log_perror(APLOG_MARK, APLOG_ERR, rv, rec->pool,
- "DBD: failed to prepare SQL statements: %s",
- (errmsg ? errmsg : "[???]"));
+ ap_log_error(APLOG_MARK, APLOG_ERR, rv, svr->server,
+ "DBD: failed to prepare SQL statements: %s",
+ (errmsg ? errmsg : "[???]"));
+
+ apr_dbd_close(rec->driver, rec->handle);
+ apr_pool_destroy(rec->pool);
+ return rv;
}
- return rv;
+ *data_ptr = rec;
+
+ return APR_SUCCESS;
}
#if APR_HAS_THREADS
-static apr_status_t dbd_setup(apr_pool_t *pool, svr_cfg *svr)
+static apr_status_t dbd_setup(apr_pool_t *pool, server_rec *s,
+ svr_cfg *svr)
{
apr_status_t rv;
@@ -397,8 +411,8 @@ static apr_status_t dbd_setup(apr_pool_t *pool, svr_cfg *svr)
*/
rv = apr_pool_create(&svr->pool, pool);
if (rv != APR_SUCCESS) {
- ap_log_perror(APLOG_MARK, APLOG_CRIT, rv, pool,
- "DBD: Failed to create reslist memory pool");
+ ap_log_error(APLOG_MARK, APLOG_CRIT, rv, s,
+ "DBD: Failed to create reslist memory pool");
return rv;
}
@@ -407,19 +421,14 @@ static apr_status_t dbd_setup(apr_pool_t *pool, svr_cfg *svr)
apr_time_from_sec(svr->exptime),
dbd_construct, dbd_destruct, svr,
svr->pool);
- if (rv == APR_SUCCESS) {
- apr_pool_cleanup_register(svr->pool, svr->reslist,
- (apr_status_t (*)(void*)) apr_reslist_destroy,
- apr_pool_cleanup_null);
- }
- else {
- ap_log_perror(APLOG_MARK, APLOG_ERR, rv, svr->pool,
- "DBD: failed to initialise");
+ if (rv != APR_SUCCESS) {
+ ap_log_error(APLOG_MARK, APLOG_ERR, rv, s,
+ "DBD: failed to initialise");
apr_pool_destroy(svr->pool);
- svr->pool = NULL;
+ return rv;
}
- return rv;
+ return APR_SUCCESS;
}
static apr_status_t dbd_setup_init(apr_pool_t *pool, server_rec *s)
@@ -440,7 +449,7 @@ static apr_status_t dbd_setup_init(apr_pool_t *pool, server_rec *s)
return APR_SUCCESS;
}
- rv = dbd_setup(pool, svr);
+ rv = dbd_setup(pool, s, svr);
if (rv == APR_SUCCESS) {
return rv;
}
@@ -451,16 +460,16 @@ static apr_status_t dbd_setup_init(apr_pool_t *pool, server_rec *s)
rv = apr_thread_mutex_create(&svr->mutex,
APR_THREAD_MUTEX_DEFAULT, pool);
if (rv != APR_SUCCESS) {
- ap_log_perror(APLOG_MARK, APLOG_CRIT, rv, pool,
- "DBD: Failed to create thread mutex");
+ ap_log_error(APLOG_MARK, APLOG_CRIT, rv, s,
+ "DBD: Failed to create thread mutex");
}
return rv;
}
-static apr_status_t dbd_setup_lock(apr_pool_t *pool, server_rec *s)
+static apr_status_t dbd_setup_lock(apr_pool_t *pool, server_rec *s,
+ svr_cfg *svr)
{
- svr_cfg *svr = ap_get_module_config(s->module_config, &dbd_module);
apr_status_t rv = APR_SUCCESS, rv2;
/* several threads could be here at the same time, all trying to
@@ -473,19 +482,19 @@ static apr_status_t dbd_setup_lock(apr_pool_t *pool, server_rec *s)
rv2 = apr_thread_mutex_lock(svr->mutex);
if (rv2 != APR_SUCCESS) {
- ap_log_perror(APLOG_MARK, APLOG_ERR, rv2, pool,
- "DBD: Failed to acquire thread mutex");
+ ap_log_error(APLOG_MARK, APLOG_ERR, rv2, s,
+ "DBD: Failed to acquire thread mutex");
return rv2;
}
if (!svr->reslist) {
- rv = dbd_setup(s->process->pool, svr);
+ rv = dbd_setup(s->process->pool, s, svr);
}
rv2 = apr_thread_mutex_unlock(svr->mutex);
if (rv2 != APR_SUCCESS) {
- ap_log_perror(APLOG_MARK, APLOG_ERR, rv2, pool,
- "DBD: Failed to release thread mutex");
+ ap_log_error(APLOG_MARK, APLOG_ERR, rv2, s,
+ "DBD: Failed to release thread mutex");
if (rv == APR_SUCCESS) {
rv = rv2;
}
@@ -513,102 +522,91 @@ DBD_DECLARE_NONSTD(void) ap_dbd_close(server_rec *s, ap_dbd_t *rec)
#endif
}
-#define arec ((ap_dbd_t*)rec)
+static apr_status_t dbd_check(apr_pool_t *pool, server_rec *s, ap_dbd_t *rec)
+{
+ svr_cfg *svr;
+ apr_status_t rv = apr_dbd_check_conn(rec->driver, pool, rec->handle);
+ const char *errmsg;
+
+ if ((rv == APR_SUCCESS) || (rv == APR_ENOTIMPL)) {
+ return APR_SUCCESS;
+ }
+
+ errmsg = apr_dbd_error(rec->driver, rec->handle, rv);
+ if (!errmsg) {
+ errmsg = "(unknown)";
+ }
+
+ svr = ap_get_module_config(s->module_config, &dbd_module);
+ ap_log_error(APLOG_MARK, APLOG_ERR, rv, s,
+ "DBD [%s] Error: %s", svr->name, errmsg);
+ return rv;
+}
-#if APR_HAS_THREADS
DBD_DECLARE_NONSTD(ap_dbd_t*) ap_dbd_open(apr_pool_t *pool, server_rec *s)
{
svr_cfg *svr = ap_get_module_config(s->module_config, &dbd_module);
- void *rec = NULL;
- apr_status_t rv = APR_SUCCESS;
- const char *errmsg;
+ ap_dbd_t *rec = NULL;
+#if APR_HAS_THREADS
+ apr_status_t rv;
+#endif
/* If nothing is configured, we shouldn't be here */
if (svr->name == no_dbdriver) {
- ap_log_perror(APLOG_MARK, APLOG_ERR, 0, pool, "DBD: not configured");
+ ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, "DBD: not configured");
return NULL;
}
if (!svr->persist) {
/* Return a once-only connection */
- rv = dbd_construct(&rec, svr, s->process->pool);
- return (rv == APR_SUCCESS) ? arec : NULL;
+ dbd_construct((void**) &rec, svr, s->process->pool);
+ return rec;
}
+#if APR_HAS_THREADS
if (!svr->reslist) {
- if (dbd_setup_lock(pool, s) != APR_SUCCESS) {
+ if (dbd_setup_lock(pool, s, svr) != APR_SUCCESS) {
return NULL;
}
}
- rv = apr_reslist_acquire(svr->reslist, &rec);
+ rv = apr_reslist_acquire(svr->reslist, (void**) &rec);
if (rv != APR_SUCCESS) {
- ap_log_perror(APLOG_MARK, APLOG_ERR, rv, pool,
- "Failed to acquire DBD connection from pool!");
+ ap_log_error(APLOG_MARK, APLOG_ERR, rv, s,
+ "Failed to acquire DBD connection from pool!");
return NULL;
}
- rv = apr_dbd_check_conn(arec->driver, pool, arec->handle);
- if ((rv != APR_SUCCESS) && (rv != APR_ENOTIMPL)) {
- errmsg = apr_dbd_error(arec->driver, arec->handle, rv);
- if (!errmsg) {
- errmsg = "(unknown)";
- }
- ap_log_perror(APLOG_MARK, APLOG_ERR, rv, pool,
- "DBD [%s] Error: %s", svr->name, errmsg );
+ if (dbd_check(pool, s, rec) != APR_SUCCESS) {
apr_reslist_invalidate(svr->reslist, rec);
return NULL;
}
- return arec;
-}
#else
-DBD_DECLARE_NONSTD(ap_dbd_t*) ap_dbd_open(apr_pool_t *pool, server_rec *s)
-{
- svr_cfg *svr = ap_get_module_config(s->module_config, &dbd_module);
- void *rec = NULL;
- apr_status_t rv = APR_SUCCESS;
- const char *errmsg;
-
- /* If nothing is configured, we shouldn't be here */
- if (svr->name == no_dbdriver) {
- ap_log_perror(APLOG_MARK, APLOG_ERR, 0, pool, "DBD: not configured");
- return NULL;
- }
-
- if (!svr->persist) {
- /* Return a once-only connection */
- rv = dbd_construct(&rec, svr, s->process->pool);
- return (rv == APR_SUCCESS) ? arec : NULL;
- }
-
/* If we have a persistent connection and it's good, we'll use it;
* since this is non-threaded, we can update without a mutex
*/
- if (svr->rec) {
- rv = apr_dbd_check_conn(svr->rec->driver, pool, svr->rec->handle);
- if ((rv != APR_SUCCESS) && (rv != APR_ENOTIMPL)) {
- errmsg = apr_dbd_error(arec->driver, arec->handle, rv);
- if (!errmsg) {
- errmsg = "(unknown)";
- }
- ap_log_perror(APLOG_MARK, APLOG_ERR, rv, pool,
- "DBD [%s] Error: %s", svr->name, errmsg);
- svr->rec = NULL;
+ rec = svr->rec;
+ if (rec) {
+ if (dbd_check(pool, s, rec) != APR_SUCCESS) {
+ apr_pool_cleanup_run(s->process->pool, rec, dbd_close);
+ rec = NULL;
}
}
/* We don't have a connection right now, so we'll open one */
- if (!svr->rec) {
- if (dbd_construct(&rec, svr, s->process->pool) == APR_SUCCESS) {
- svr->rec = arec ;
- apr_pool_cleanup_register(s->process->pool, svr->rec,
+ if (!rec) {
+ dbd_construct((void**) &rec, svr, s->process->pool);
+ svr->rec = rec;
+
+ if (rec) {
+ apr_pool_cleanup_register(s->process->pool, rec,
dbd_close, apr_pool_cleanup_null);
}
}
+#endif
- return svr->rec;
+ return rec;
}
-#endif
#if APR_HAS_THREADS
typedef struct {
@@ -710,7 +708,7 @@ DBD_DECLARE_NONSTD(ap_dbd_t *) ap_dbd_acquire(request_rec *r)
ap_set_module_config(r->request_config, &dbd_module, rec);
/* if persist then ap_dbd_open registered cleanup on proc pool */
if (!svr->persist) {
- apr_pool_cleanup_register(r->pool, svr->rec, dbd_close,
+ apr_pool_cleanup_register(r->pool, rec, dbd_close,
apr_pool_cleanup_null);
}
}
@@ -732,7 +730,7 @@ DBD_DECLARE_NONSTD(ap_dbd_t *) ap_dbd_cacquire(conn_rec *c)
ap_set_module_config(c->conn_config, &dbd_module, rec);
/* if persist then ap_dbd_open registered cleanup on proc pool */
if (!svr->persist) {
- apr_pool_cleanup_register(c->pool, svr->rec, dbd_close,
+ apr_pool_cleanup_register(c->pool, rec, dbd_close,
apr_pool_cleanup_null);
}
}