From: Nick Porter Date: Fri, 26 Apr 2024 15:12:52 +0000 (+0100) Subject: Change function signature of rlm_sql_query() to be unlang_function_t X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a57bde89e2e375ac65a4dc9091fd2f0d6ed963b6;p=thirdparty%2Ffreeradius-server.git Change function signature of rlm_sql_query() to be unlang_function_t --- diff --git a/src/modules/rlm_sql/rlm_sql.c b/src/modules/rlm_sql/rlm_sql.c index a545f054c5b..b82e242564b 100644 --- a/src/modules/rlm_sql/rlm_sql.c +++ b/src/modules/rlm_sql/rlm_sql.c @@ -403,6 +403,8 @@ static xlat_action_t sql_xlat(TALLOC_CTX *ctx, fr_dcursor_t *out, fr_value_box_t *arg = fr_value_box_list_head(in); fr_value_box_t *vb = NULL; bool fetched = false; + fr_sql_query_t *query_ctx = NULL; + rlm_rcode_t p_result; handle = fr_pool_connection_get(inst->pool, request); /* connection pool should produce error */ if (!handle) return XLAT_ACTION_FAIL; @@ -427,19 +429,21 @@ static xlat_action_t sql_xlat(TALLOC_CTX *ctx, fr_dcursor_t *out, (strncasecmp(p, "delete", 6) == 0)) { int numaffected; - rcode = rlm_sql_query(inst, request, &handle, arg->vb_strvalue); - if (rcode != RLM_SQL_OK) { + MEM(query_ctx = fr_sql_query_alloc(unlang_interpret_frame_talloc_ctx(request), inst, handle, + arg->vb_strvalue, SQL_QUERY_OTHER)); + rlm_sql_query(&p_result, NULL, request, query_ctx); + if (query_ctx->rcode != RLM_SQL_OK) { query_error: - RERROR("SQL query failed: %s", fr_table_str_by_value(sql_rcode_description_table, rcode, "")); + RERROR("SQL query failed: %s", fr_table_str_by_value(sql_rcode_description_table, + query_ctx ? query_ctx->rcode : rcode, "")); ret = XLAT_ACTION_FAIL; goto finish; } - numaffected = (inst->driver->sql_affected_rows)(handle, &inst->config); + numaffected = (inst->driver->sql_affected_rows)(query_ctx->handle, &inst->config); if (numaffected < 1) { RDEBUG2("SQL query affected no rows"); - (inst->driver->sql_finish_query)(handle, &inst->config); goto finish; } @@ -448,8 +452,6 @@ static xlat_action_t sql_xlat(TALLOC_CTX *ctx, fr_dcursor_t *out, fr_value_box_uint32(vb, NULL, (uint32_t)numaffected, false); fr_dcursor_append(out, vb); - (inst->driver->sql_finish_query)(handle, &inst->config); - goto finish; } /* else it's a SELECT statement */ @@ -492,6 +494,8 @@ finish_query: (inst->driver->sql_finish_select_query)(handle, &inst->config); finish: + handle = query_ctx->handle ? query_ctx->handle : handle; + talloc_free(query_ctx); fr_pool_connection_release(inst->pool, request, handle); return ret; @@ -1503,9 +1507,9 @@ static unlang_action_t mod_sql_redundant_resume(rlm_rcode_t *p_result, UNUSED in sql_redundant_call_env_t *call_env = redundant_ctx->call_env; rlm_sql_t const *inst = redundant_ctx->inst; fr_value_box_t *query; - int sql_ret; int numaffected = 0; tmpl_t *next_query; + fr_sql_query_t *query_ctx; query = fr_value_box_list_pop_head(&redundant_ctx->query); if (!query) RETURN_MODULE_FAIL; @@ -1514,12 +1518,15 @@ static unlang_action_t mod_sql_redundant_resume(rlm_rcode_t *p_result, UNUSED in rlm_sql_query_log(inst, call_env->filename.vb_strvalue, query->vb_strvalue); } - sql_ret = rlm_sql_query(inst, request, &redundant_ctx->handle, query->vb_strvalue); + MEM(query_ctx = fr_sql_query_alloc(unlang_interpret_frame_talloc_ctx(request), inst, redundant_ctx->handle, + query->vb_strvalue, SQL_QUERY_SELECT)); + + rlm_sql_query(p_result, NULL, request, query_ctx); talloc_free(query); - RDEBUG2("SQL query returned: %s", fr_table_str_by_value(sql_rcode_description_table, sql_ret, "")); + RDEBUG2("SQL query returned: %s", fr_table_str_by_value(sql_rcode_description_table, query_ctx->rcode, "")); - switch (sql_ret) { + switch (query_ctx->rcode) { /* * Query was a success! Now we just need to check if it did anything. */ @@ -1550,6 +1557,9 @@ static unlang_action_t mod_sql_redundant_resume(rlm_rcode_t *p_result, UNUSED in */ case RLM_SQL_ALT_QUERY: goto next; + + case RLM_SQL_NO_MORE_ROWS: + break; } fr_assert(redundant_ctx->handle); @@ -1558,7 +1568,7 @@ static unlang_action_t mod_sql_redundant_resume(rlm_rcode_t *p_result, UNUSED in * counted as successful. */ numaffected = (inst->driver->sql_affected_rows)(redundant_ctx->handle, &inst->config); - (inst->driver->sql_finish_query)(redundant_ctx->handle, &inst->config); + talloc_free(query_ctx); RDEBUG2("%i record(s) updated", numaffected); if (numaffected > 0) RETURN_MODULE_OK; /* A query succeeded, were done! */ diff --git a/src/modules/rlm_sql/rlm_sql.h b/src/modules/rlm_sql/rlm_sql.h index a34c361984e..3cc8d63c84e 100644 --- a/src/modules/rlm_sql/rlm_sql.h +++ b/src/modules/rlm_sql/rlm_sql.h @@ -31,6 +31,7 @@ RCSIDH(rlm_sql_h, "$Id$") #include #include #include +#include #define FR_ITEM_CHECK 0 #define FR_ITEM_REPLY 1 @@ -208,7 +209,7 @@ struct sql_inst { xlat_escape_legacy_t sql_escape_func; fr_value_box_escape_t box_escape_func; - sql_rcode_t (*query)(rlm_sql_t const *inst, request_t *request, rlm_sql_handle_t **handle, char const *query); + unlang_function_t query; sql_rcode_t (*select)(rlm_sql_t const *inst, request_t *request, rlm_sql_handle_t **handle, char const *query); sql_rcode_t (*fetch_row)(rlm_sql_row_t *out, rlm_sql_t const *inst, request_t *request, rlm_sql_handle_t **handle); fr_sql_query_t *(*query_alloc)(TALLOC_CTX *ctx, rlm_sql_t const *inst, rlm_sql_handle_t *handle, char const *query_str, fr_sql_query_type_t type); @@ -221,7 +222,7 @@ void *sql_mod_conn_create(TALLOC_CTX *ctx, void *instance, fr_time_delta_t time int sql_get_map_list(TALLOC_CTX *ctx, rlm_sql_t const *inst, request_t *request, rlm_sql_handle_t **handle, map_list_t *out, char const *query, fr_dict_attr_t const *list); void rlm_sql_query_log(rlm_sql_t const *inst, char const *filename, char const *query) CC_HINT(nonnull); sql_rcode_t rlm_sql_select_query(rlm_sql_t const *inst, request_t *request, rlm_sql_handle_t **handle, char const *query) CC_HINT(nonnull (1, 3, 4)); -sql_rcode_t rlm_sql_query(rlm_sql_t const *inst, request_t *request, rlm_sql_handle_t **handle, char const *query) CC_HINT(nonnull (1, 3, 4)); +unlang_action_t rlm_sql_query(rlm_rcode_t *p_result, int *priority, request_t *request, void *uctx); sql_rcode_t rlm_sql_fetch_row(rlm_sql_row_t *out, rlm_sql_t const *inst, request_t *request, rlm_sql_handle_t **handle); void rlm_sql_print_error(rlm_sql_t const *inst, request_t *request, rlm_sql_handle_t *handle, bool force_debug); fr_sql_query_t *fr_sql_query_alloc(TALLOC_CTX *ctx, rlm_sql_t const *inst, rlm_sql_handle_t *handle, char const *query_str, fr_sql_query_type_t type); diff --git a/src/modules/rlm_sql/sql.c b/src/modules/rlm_sql/sql.c index a0af6d73280..f2f0ff73c99 100644 --- a/src/modules/rlm_sql/sql.c +++ b/src/modules/rlm_sql/sql.c @@ -410,29 +410,31 @@ fr_sql_query_t *fr_sql_query_alloc(TALLOC_CTX *ctx, rlm_sql_t const *inst, rlm_s * @note Caller must call ``(inst->driver->sql_finish_query)(handle, &inst->config);`` * after they're done with the result. * - * @param handle to query the database with. *handle should not be NULL, as this indicates - * previous reconnection attempt has failed. - * @param request Current request. - * @param inst #rlm_sql_t instance data. - * @param query to execute. Should not be zero length. - * @return + * The rcode within the query context is updated to * - #RLM_SQL_OK on success. - * - #RLM_SQL_RECONNECT if a new handle is required (also sets *handle = NULL). + * - #RLM_SQL_RECONNECT if a new handle is required (also sets the handle to NULL). * - #RLM_SQL_QUERY_INVALID, #RLM_SQL_ERROR on invalid query or connection error. * - #RLM_SQL_ALT_QUERY on constraints violation. + * + * @param p_result Result of current module call. + * @param priority Unused. + * @param request Current request. + * @param uctx query context containing query to execute. + * @return an unlang_action_t. */ -sql_rcode_t rlm_sql_query(rlm_sql_t const *inst, request_t *request, rlm_sql_handle_t **handle, char const *query) +unlang_action_t rlm_sql_query(rlm_rcode_t *p_result, UNUSED int *priority, request_t *request, void *uctx) { - int ret = RLM_SQL_ERROR; - int i, count; + fr_sql_query_t *query_ctx = talloc_get_type_abort(uctx, fr_sql_query_t); + rlm_sql_t const *inst = query_ctx->inst; + int i, count; /* Caller should check they have a valid handle */ - fr_assert(*handle); + fr_assert(query_ctx->handle); /* There's no query to run, return an error */ - if (query[0] == '\0') { + if (query_ctx->query_str[0] == '\0') { if (request) REDEBUG("Zero length query"); - return RLM_SQL_QUERY_INVALID; + RETURN_MODULE_INVALID; } /* @@ -445,21 +447,22 @@ sql_rcode_t rlm_sql_query(rlm_sql_t const *inst, request_t *request, rlm_sql_han * a new connection, then give up. */ for (i = 0; i < (count + 1); i++) { - ROPTIONAL(RDEBUG2, DEBUG2, "Executing query: %s", query); + ROPTIONAL(RDEBUG2, DEBUG2, "Executing query: %s", query_ctx->query_str); - ret = (inst->driver->sql_query)(*handle, &inst->config, query); - switch (ret) { + query_ctx->rcode = (inst->driver->sql_query)(query_ctx->handle, &inst->config, query_ctx->query_str); + query_ctx->status = SQL_QUERY_SUBMITTED; + switch (query_ctx->rcode) { case RLM_SQL_OK: - break; + RETURN_MODULE_OK; /* * Run through all available sockets until we exhaust all existing * sockets in the pool and fail to establish a *new* connection. */ case RLM_SQL_RECONNECT: - *handle = fr_pool_connection_reconnect(inst->pool, request, *handle); + query_ctx->handle = fr_pool_connection_reconnect(inst->pool, request, query_ctx->handle); /* Reconnection failed */ - if (!*handle) return RLM_SQL_RECONNECT; + if (!query_ctx->handle) RETURN_MODULE_FAIL; /* Reconnection succeeded, try again with the new handle */ continue; @@ -467,9 +470,9 @@ sql_rcode_t rlm_sql_query(rlm_sql_t const *inst, request_t *request, rlm_sql_han * These are bad and should make rlm_sql return invalid */ case RLM_SQL_QUERY_INVALID: - rlm_sql_print_error(inst, request, *handle, false); - (inst->driver->sql_finish_query)(*handle, &inst->config); - break; + rlm_sql_print_error(inst, request, query_ctx->handle, false); + (inst->driver->sql_finish_query)(query_ctx->handle, &inst->config); + RETURN_MODULE_INVALID; /* * Server or client errors. @@ -482,29 +485,31 @@ sql_rcode_t rlm_sql_query(rlm_sql_t const *inst, request_t *request, rlm_sql_han */ case RLM_SQL_ERROR: if (inst->driver->flags & RLM_SQL_RCODE_FLAGS_ALT_QUERY) { - rlm_sql_print_error(inst, request, *handle, false); - (inst->driver->sql_finish_query)(*handle, &inst->config); - break; + rlm_sql_print_error(inst, request, query_ctx->handle, false); + (inst->driver->sql_finish_query)(query_ctx->handle, &inst->config); + RETURN_MODULE_FAIL; } - ret = RLM_SQL_ALT_QUERY; FALL_THROUGH; /* * Driver suggested using an alternative query */ case RLM_SQL_ALT_QUERY: - rlm_sql_print_error(inst, request, *handle, true); - (inst->driver->sql_finish_query)(*handle, &inst->config); + rlm_sql_print_error(inst, request, query_ctx->handle, true); + (inst->driver->sql_finish_query)(query_ctx->handle, &inst->config); break; + default: + break; } - return ret; + RETURN_MODULE_OK; } ROPTIONAL(RERROR, ERROR, "Hit reconnection limit"); - return RLM_SQL_ERROR; + query_ctx->rcode = RLM_SQL_ERROR; + RETURN_MODULE_FAIL; } /** Call the driver's sql_select_query method, reconnecting if necessary. diff --git a/src/modules/rlm_sqlippool/rlm_sqlippool.c b/src/modules/rlm_sqlippool/rlm_sqlippool.c index f4666eafa30..f0b193cb8c9 100644 --- a/src/modules/rlm_sqlippool/rlm_sqlippool.c +++ b/src/modules/rlm_sqlippool/rlm_sqlippool.c @@ -131,7 +131,9 @@ static void *sql_escape_uctx_alloc(request_t *request, void const *uctx) static int sqlippool_command(char const *query, rlm_sql_handle_t **handle, rlm_sql_t const *sql, request_t *request) { - int ret, affected; + int affected; + fr_sql_query_t *query_ctx; + rlm_rcode_t p_result; /* * If we don't have a command, do nothing. @@ -143,17 +145,20 @@ static int sqlippool_command(char const *query, rlm_sql_handle_t **handle, */ if (!handle || !*handle) return -1; - ret = sql->query(sql, request, handle, query); - if (ret < 0) return -1; + query_ctx = sql->query_alloc(unlang_interpret_frame_talloc_ctx(request), sql, *handle, query, SQL_QUERY_SELECT); + + sql->query(&p_result, NULL, request, query_ctx); + *handle = query_ctx->handle; + if (query_ctx->rcode < 0) return -1; /* * No handle, we can't continue. */ - if (!*handle) return -1; + if (!query_ctx->handle) return -1; affected = (sql->driver->sql_affected_rows)(*handle, &sql->config); - (sql->driver->sql_finish_query)(*handle, &sql->config); + talloc_free(query_ctx); return affected; }