On Fri, 2007-10-19 at 15:05 +1000, Bojan Smojver wrote:
> Yeah, that's been the problem of the 1.2 API. In 1.3, we cannot do it
> with the same function (API/ABI backward compatibility), but we can
> introduce another function (e.g. apr_dbd_open_ex()) that does return an
> error.
Perhaps something like this (compiles but untested)...
--
Bojan
Index: include/apr_dbd.h
===================================================================
--- include/apr_dbd.h (revision 586254)
+++ include/apr_dbd.h (working copy)
@@ -105,6 +105,42 @@
APU_DECLARE(apr_status_t) apr_dbd_get_driver(apr_pool_t *pool, const char *name,
const apr_dbd_driver_t **driver);
+/** apr_dbd_open_ex: open a connection to a backend
+ *
+ * @param pool - working pool
+ * @param params - arguments to driver (implementation-dependent)
+ * @param handle - pointer to handle to return
+ * @param driver - driver struct.
+ * @param error - descriptive error.
+ * @return APR_SUCCESS for success
+ * @return APR_EGENERAL if driver exists but connection failed
+ * @remarks PostgreSQL: the params is passed directly to the PQconnectdb()
+ * function (check PostgreSQL documentation for more details on the syntax).
+ * @remarks SQLite2: the params is split on a colon, with the first part used
+ * as the filename and second part converted to an integer and used as file
+ * mode.
+ * @remarks SQLite3: the params is passed directly to the sqlite3_open()
+ * function as a filename to be opened (check SQLite3 documentation for more
+ * details).
+ * @remarks Oracle: the params can have "user", "pass", "dbname" and "server"
+ * keys, each followed by an equal sign and a value. Such key/value pairs can
+ * be delimited by space, CR, LF, tab, semicolon, vertical bar or comma.
+ * @remarks MySQL: the params can have "host", "port", "user", "pass",
+ * "dbname", "sock", "flags" "fldsz" and "group" keys, each followed by an
+ * equal sign and a value. Such key/value pairs can be delimited by space,
+ * CR, LF, tab, semicolon, vertical bar or comma. For now, "flags" can only
+ * recognise CLIENT_FOUND_ROWS (check MySQL manual for details). The value
+ * associated with "fldsz" determines maximum amount of memory (in bytes) for
+ * each of the fields in the result set of prepared statements. By default,
+ * this value is 1 MB. The value associated with "group" determines which
+ * group from configuration file to use (see MYSQL_READ_DEFAULT_GROUP option
+ * of mysql_options() in MySQL manual).
+ */
+APU_DECLARE(apr_status_t) apr_dbd_open_ex(const apr_dbd_driver_t *driver,
+ apr_pool_t *pool, const char *params,
+ apr_dbd_t **handle,
+ const char **error);
+
/** apr_dbd_open: open a connection to a backend
*
* @param pool - working pool
Index: include/private/apr_dbd_internal.h
===================================================================
--- include/private/apr_dbd_internal.h (revision 586254)
+++ include/private/apr_dbd_internal.h (working copy)
@@ -62,10 +62,12 @@
* a lifetime other than a request
*
* @param pool - a pool to use for error messages (if any).
- * @param s - server rec managing the underlying connection/pool.
+ * @param params - connection parameters.
+ * @param error - descriptive error.
* @return database handle, or NULL on error.
*/
- apr_dbd_t *(*open)(apr_pool_t *pool, const char *params);
+ apr_dbd_t *(*open)(apr_pool_t *pool, const char *params,
+ const char **error);
/** check_conn: check status of a database connection
*
Index: dbd/apr_dbd_sqlite2.c
===================================================================
--- dbd/apr_dbd_sqlite2.c (revision 586254)
+++ dbd/apr_dbd_sqlite2.c (working copy)
@@ -446,8 +446,15 @@
return trans->mode = (mode & TXN_MODE_BITS);
}
-static apr_dbd_t *dbd_sqlite_open(apr_pool_t * pool, const char *params_)
+static apr_status_t error_free(void *data)
{
+ free(data);
+ return APR_SUCCESS;
+}
+
+static apr_dbd_t *dbd_sqlite_open(apr_pool_t * pool, const char *params_,
+ const char **error)
+{
apr_dbd_t *sql;
sqlite *conn = NULL;
char *perm;
@@ -465,8 +472,20 @@
iperms = atoi(perm);
}
- conn = sqlite_open(params, iperms, NULL);
+ if (error) {
+ *error = NULL;
+ conn = sqlite_open(params, iperms, (char **)error);
+
+ if (*error) {
+ apr_pool_cleanup_register(pool, *error, error_free,
+ apr_pool_cleanup_null);
+ }
+ }
+ else {
+ conn = sqlite_open(params, iperms, NULL);
+ }
+
sql = apr_pcalloc(pool, sizeof(*sql));
sql->conn = conn;
Index: dbd/apr_dbd_sqlite3.c
===================================================================
--- dbd/apr_dbd_sqlite3.c (revision 586254)
+++ dbd/apr_dbd_sqlite3.c (working copy)
@@ -814,7 +814,8 @@
return trans->mode = (mode & TXN_MODE_BITS);
}
-static apr_dbd_t *dbd_sqlite3_open(apr_pool_t *pool, const char *params)
+static apr_dbd_t *dbd_sqlite3_open(apr_pool_t *pool, const char *params,
+ const char **error)
{
apr_dbd_t *sql = NULL;
sqlite3 *conn = NULL;
@@ -823,6 +824,9 @@
return NULL;
sqlres = sqlite3_open(params, &conn);
if (sqlres != SQLITE_OK) {
+ if (error) {
+ *error = apr_pstrdup(pool, sqlite3_errmsg(conn));
+ }
sqlite3_close(conn);
return NULL;
}
Index: dbd/apr_dbd_oracle.c
===================================================================
--- dbd/apr_dbd_oracle.c (revision 586254)
+++ dbd/apr_dbd_oracle.c (working copy)
@@ -437,10 +437,11 @@
#endif
}
-static apr_dbd_t *dbd_oracle_open(apr_pool_t *pool, const char *params)
+static apr_dbd_t *dbd_oracle_open(apr_pool_t *pool, const char *params,
+ const char **error)
{
apr_dbd_t *ret = apr_pcalloc(pool, sizeof(apr_dbd_t));
- int_errorcode;
+ int errorcode;
char *BLANK = "";
struct {
@@ -519,6 +520,9 @@
printf("OPEN ERROR %d (alloc svr): %s\n", ret->status, ret->buf);
break;
#else
+ *error = apr_pcalloc(pool, 200);
+ OCIErrorGet(ret->err, 1, NULL, &errorcode, (unsigned char*)(*error),
+ 200, OCI_HTYPE_ERROR);
return NULL;
#endif
case OCI_SUCCESS:
@@ -535,6 +539,9 @@
printf("OPEN ERROR %d (alloc svc): %s\n", ret->status, ret->buf);
break;
#else
+ *error = apr_pcalloc(pool, 200);
+ OCIErrorGet(ret->err, 1, NULL, &errorcode, (unsigned char*)(*error),
+ 200, OCI_HTYPE_ERROR);
return NULL;
#endif
case OCI_SUCCESS:
@@ -555,6 +562,9 @@
printf("OPEN ERROR: %s\n", ret->buf);
break;
#else
+ *error = apr_pcalloc(pool, 200);
+ OCIErrorGet(ret->err, 1, NULL, &errorcode, (unsigned char*)(*error),
+ 200, OCI_HTYPE_ERROR);
return NULL;
#endif
case OCI_SUCCESS:
@@ -571,6 +581,9 @@
printf("OPEN ERROR %d (server attach): %s\n", ret->status, ret->buf);
break;
#else
+ *error = apr_pcalloc(pool, 200);
+ OCIErrorGet(ret->err, 1, NULL, &errorcode, (unsigned char*)(*error),
+ 200, OCI_HTYPE_ERROR);
return NULL;
#endif
case OCI_SUCCESS:
@@ -586,6 +599,9 @@
printf("OPEN ERROR %d (attr set): %s\n", ret->status, ret->buf);
break;
#else
+ *error = apr_pcalloc(pool, 200);
+ OCIErrorGet(ret->err, 1, NULL, &errorcode, (unsigned char*)(*error),
+ 200, OCI_HTYPE_ERROR);
return NULL;
#endif
case OCI_SUCCESS:
@@ -601,6 +617,9 @@
printf("OPEN ERROR %d (alloc auth): %s\n", ret->status, ret->buf);
break;
#else
+ *error = apr_pcalloc(pool, 200);
+ OCIErrorGet(ret->err, 1, NULL, &errorcode, (unsigned char*)(*error),
+ 200, OCI_HTYPE_ERROR);
return NULL;
#endif
case OCI_SUCCESS:
@@ -616,6 +635,9 @@
printf("OPEN ERROR %d (attr username): %s\n", ret->status, ret->buf);
break;
#else
+ *error = apr_pcalloc(pool, 200);
+ OCIErrorGet(ret->err, 1, NULL, &errorcode, (unsigned char*)(*error),
+ 200, OCI_HTYPE_ERROR);
return NULL;
#endif
case OCI_SUCCESS:
@@ -631,6 +653,9 @@
printf("OPEN ERROR %d (attr password): %s\n", ret->status, ret->buf);
break;
#else
+ *error = apr_pcalloc(pool, 200);
+ OCIErrorGet(ret->err, 1, NULL, &errorcode, (unsigned char*)(*error),
+ 200, OCI_HTYPE_ERROR);
return NULL;
#endif
case OCI_SUCCESS:
@@ -646,6 +671,9 @@
printf("OPEN ERROR %d (session begin): %s\n", ret->status, ret->buf);
break;
#else
+ *error = apr_pcalloc(pool, 200);
+ OCIErrorGet(ret->err, 1, NULL, &errorcode, (unsigned char*)(*error),
+ 200, OCI_HTYPE_ERROR);
return NULL;
#endif
case OCI_SUCCESS:
@@ -660,6 +688,9 @@
sizeof(ret->buf), OCI_HTYPE_ERROR);
printf("OPEN ERROR %d (attr session): %s\n", ret->status, ret->buf);
#else
+ *error = apr_pcalloc(pool, 200);
+ OCIErrorGet(ret->err, 1, NULL, &errorcode, (unsigned char*)(*error),
+ 200, OCI_HTYPE_ERROR);
return NULL;
#endif
break;
Index: dbd/apr_dbd_mysql.c
===================================================================
--- dbd/apr_dbd_mysql.c (revision 586254)
+++ dbd/apr_dbd_mysql.c (working copy)
@@ -1064,7 +1064,8 @@
return trans->mode = (mode & TXN_MODE_BITS);
}
-static apr_dbd_t *dbd_mysql_open(apr_pool_t *pool, const char *params)
+static apr_dbd_t *dbd_mysql_open(apr_pool_t *pool, const char *params,
+ const char **error)
{
static const char *const delims = " \r\n\t;|,";
const char *ptr;
@@ -1155,6 +1156,9 @@
fields[5].value, flags);
if(real_conn == NULL) {
+ if (error) {
+ *error = apr_pstrdup(pool, mysql_error(sql->conn));
+ }
mysql_close(sql->conn);
return NULL;
}
Index: dbd/apr_dbd_pgsql.c
===================================================================
--- dbd/apr_dbd_pgsql.c (revision 586254)
+++ dbd/apr_dbd_pgsql.c (working copy)
@@ -1176,7 +1176,8 @@
return trans->mode = (mode & TXN_MODE_BITS);
}
-static apr_dbd_t *dbd_pgsql_open(apr_pool_t *pool, const char *params)
+static apr_dbd_t *dbd_pgsql_open(apr_pool_t *pool, const char *params,
+ const char **error)
{
apr_dbd_t *sql;
@@ -1187,6 +1188,9 @@
* liable to segfault, so just close it out now. it would be nice
* if we could give an indication of why we failed to connect... */
if (PQstatus(conn) != CONNECTION_OK) {
+ if (error) {
+ *error = apr_pstrdup(pool, PQerrorMessage(conn));
+ }
PQfinish(conn);
return NULL;
}
Index: dbd/apr_dbd.c
===================================================================
--- dbd/apr_dbd.c (revision 586254)
+++ dbd/apr_dbd.c (working copy)
@@ -183,22 +183,33 @@
return rv;
}
-APU_DECLARE(apr_status_t) apr_dbd_open(const apr_dbd_driver_t *driver,
- apr_pool_t *pool, const char *params,
- apr_dbd_t **handle)
+APU_DECLARE(apr_status_t) apr_dbd_open_ex(const apr_dbd_driver_t *driver,
+ apr_pool_t *pool, const char *params,
+ apr_dbd_t **handle,
+ const char **error)
{
apr_status_t rv;
- *handle = (driver->open)(pool, params);
+ *handle = (driver->open)(pool, params, error);
if (*handle == NULL) {
return APR_EGENERAL;
}
rv = apr_dbd_check_conn(driver, pool, *handle);
if ((rv != APR_SUCCESS) && (rv != APR_ENOTIMPL)) {
+ /* XXX: rv is APR error code, but apr_dbd_error() takes int! */
+ if (error) {
+ *error = apr_dbd_error(driver, *handle, rv);
+ }
apr_dbd_close(driver, *handle);
return APR_EGENERAL;
}
return APR_SUCCESS;
}
+APU_DECLARE(apr_status_t) apr_dbd_open(const apr_dbd_driver_t *driver,
+ apr_pool_t *pool, const char *params,
+ apr_dbd_t **handle)
+{
+ return apr_dbd_open_ex(driver,pool,params,handle,NULL);
+}
APU_DECLARE(int) apr_dbd_transaction_start(const apr_dbd_driver_t *driver,
apr_pool_t *pool, apr_dbd_t *handle,
apr_dbd_transaction_t **trans)
