Call finish_select_query if we experience an error retrieving the result
[freeradius.git] / src / modules / rlm_sql / rlm_sql.c
index ca818d9..c703658 100644 (file)
@@ -1,7 +1,8 @@
 /*
  *   This program is is free software; you can redistribute it and/or modify
- *   it under the terms of the GNU General Public License, version 2 of the
- *   License as published by the Free Software Foundation.
+ *   it under the terms of the GNU General Public License as published by
+ *   the Free Software Foundation; either version 2 of the License, or (at
+ *   your option) any later version.
  *
  *   This program is distributed in the hope that it will be useful,
  *   but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ -18,7 +19,7 @@
  * @file rlm_sql.c
  * @brief Implements SQL 'users' file, and SQL accounting.
  *
- * @copyright 2012  Arran Cudbard-Bell <a.cudbardb@freeradius.org>
+ * @copyright 2012-2014  Arran Cudbard-Bell <a.cudbardb@freeradius.org>
  * @copyright 2000,2006  The FreeRADIUS server project
  * @copyright 2000  Mike Machado <mike@innercite.com>
  * @copyright 2000  Alan DeKok <aland@ox.org>
@@ -41,10 +42,9 @@ RCSID("$Id$")
  *     So we can do pass2 xlat checks on the queries.
  */
 static const CONF_PARSER query_config[] = {
-
        { "query", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_XLAT | PW_TYPE_MULTI, rlm_sql_config_t, accounting.query), NULL },
 
-       {NULL, -1, 0, NULL, NULL}
+       CONF_PARSER_TERMINATOR
 };
 
 /*
@@ -52,40 +52,36 @@ static const CONF_PARSER query_config[] = {
  *     helps the average case.
  */
 static const CONF_PARSER type_config[] = {
-
        { "accounting-on", FR_CONF_POINTER(PW_TYPE_SUBSECTION, NULL), (void const *) query_config },
        { "accounting-off", FR_CONF_POINTER(PW_TYPE_SUBSECTION, NULL), (void const *) query_config },
        { "start", FR_CONF_POINTER(PW_TYPE_SUBSECTION, NULL), (void const *) query_config },
        { "interim-update", FR_CONF_POINTER(PW_TYPE_SUBSECTION, NULL), (void const *) query_config },
        { "stop", FR_CONF_POINTER(PW_TYPE_SUBSECTION, NULL), (void const *) query_config },
 
-       {NULL, -1, 0, NULL, NULL}
+       CONF_PARSER_TERMINATOR
 };
 
-
-
 static const CONF_PARSER acct_config[] = {
        { "reference", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_XLAT, rlm_sql_config_t, accounting.reference), ".query" },
-       { "logfile", FR_CONF_OFFSET(PW_TYPE_STRING, rlm_sql_config_t, accounting.logfile), NULL },
+       { "logfile", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_XLAT, rlm_sql_config_t, accounting.logfile), NULL },
 
        { "type", FR_CONF_POINTER(PW_TYPE_SUBSECTION, NULL), (void const *) type_config },
 
-       {NULL, -1, 0, NULL, NULL}
+       CONF_PARSER_TERMINATOR
 };
 
 static const CONF_PARSER postauth_config[] = {
        { "reference", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_XLAT, rlm_sql_config_t, postauth.reference), ".query" },
-       { "logfile", FR_CONF_OFFSET(PW_TYPE_STRING, rlm_sql_config_t, postauth.logfile), NULL },
+       { "logfile", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_XLAT, rlm_sql_config_t, postauth.logfile), NULL },
 
        { "query", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_XLAT | PW_TYPE_MULTI, rlm_sql_config_t, postauth.query), NULL },
-
-       {NULL, -1, 0, NULL, NULL}
+       CONF_PARSER_TERMINATOR
 };
 
 static const CONF_PARSER module_config[] = {
        { "driver", FR_CONF_OFFSET(PW_TYPE_STRING, rlm_sql_config_t, sql_driver_name), "rlm_sql_null" },
-       { "server", FR_CONF_OFFSET(PW_TYPE_STRING, rlm_sql_config_t, sql_server), "localhost" },
-       { "port", FR_CONF_OFFSET(PW_TYPE_STRING, rlm_sql_config_t, sql_port), "" },
+       { "server", FR_CONF_OFFSET(PW_TYPE_STRING, rlm_sql_config_t, sql_server), "" }, /* Must be zero length so drivers can determine if it was set */
+       { "port", FR_CONF_OFFSET(PW_TYPE_INTEGER, rlm_sql_config_t, sql_port), "0" },
        { "login", FR_CONF_OFFSET(PW_TYPE_STRING, rlm_sql_config_t, sql_login), "" },
        { "password", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_SECRET, rlm_sql_config_t, sql_password), "" },
        { "radius_db", FR_CONF_OFFSET(PW_TYPE_STRING, rlm_sql_config_t, sql_db), "radius" },
@@ -93,24 +89,24 @@ static const CONF_PARSER module_config[] = {
        { "read_profiles", FR_CONF_OFFSET(PW_TYPE_BOOLEAN, rlm_sql_config_t, read_profiles), "yes" },
        { "readclients", FR_CONF_OFFSET(PW_TYPE_BOOLEAN | PW_TYPE_DEPRECATED, rlm_sql_config_t, do_clients), NULL },
        { "read_clients", FR_CONF_OFFSET(PW_TYPE_BOOLEAN, rlm_sql_config_t, do_clients), "no" },
-       { "deletestalesessions", FR_CONF_OFFSET(PW_TYPE_BOOLEAN | PW_TYPE_DEPRECATED, rlm_sql_config_t, deletestalesessions), NULL },
-       { "delete_stale_sessions", FR_CONF_OFFSET(PW_TYPE_BOOLEAN, rlm_sql_config_t, deletestalesessions), "yes" },
-       { "sql_user_name", FR_CONF_OFFSET(PW_TYPE_STRING, rlm_sql_config_t, query_user), "" },
-       { "logfile", FR_CONF_OFFSET(PW_TYPE_STRING, rlm_sql_config_t, logfile), NULL },
+       { "deletestalesessions", FR_CONF_OFFSET(PW_TYPE_BOOLEAN | PW_TYPE_DEPRECATED, rlm_sql_config_t, delete_stale_sessions), NULL },
+       { "delete_stale_sessions", FR_CONF_OFFSET(PW_TYPE_BOOLEAN, rlm_sql_config_t, delete_stale_sessions), "yes" },
+       { "sql_user_name", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_XLAT, rlm_sql_config_t, query_user), "" },
+       { "logfile", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_XLAT, rlm_sql_config_t, logfile), NULL },
        { "default_user_profile", FR_CONF_OFFSET(PW_TYPE_STRING, rlm_sql_config_t, default_profile), "" },
        { "nas_query", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_DEPRECATED, rlm_sql_config_t, client_query), NULL },
        { "client_query", FR_CONF_OFFSET(PW_TYPE_STRING, rlm_sql_config_t, client_query), "SELECT id,nasname,shortname,type,secret FROM nas" },
-       { "open_query", FR_CONF_OFFSET(PW_TYPE_STRING, rlm_sql_config_t, open_query), NULL },
+       { "open_query", FR_CONF_OFFSET(PW_TYPE_STRING, rlm_sql_config_t, connect_query), NULL },
 
        { "authorize_check_query", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_XLAT, rlm_sql_config_t, authorize_check_query), NULL },
        { "authorize_reply_query", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_XLAT, rlm_sql_config_t, authorize_reply_query), NULL },
 
-       { "authorize_group_check_query", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_XLAT, rlm_sql_config_t, authorize_group_check_query), "" },
-       { "authorize_group_reply_query", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_XLAT, rlm_sql_config_t, authorize_group_reply_query), "" },
+       { "authorize_group_check_query", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_XLAT, rlm_sql_config_t, authorize_group_check_query), NULL },
+       { "authorize_group_reply_query", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_XLAT, rlm_sql_config_t, authorize_group_reply_query), NULL },
        { "group_membership_query", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_XLAT, rlm_sql_config_t, groupmemb_query), NULL },
 #ifdef WITH_SESSION_MGMT
-       { "simul_count_query", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_XLAT, rlm_sql_config_t, simul_count_query), "" },
-       { "simul_verify_query", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_XLAT, rlm_sql_config_t, simul_verify_query), "" },
+       { "simul_count_query", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_XLAT, rlm_sql_config_t, simul_count_query), NULL },
+       { "simul_verify_query", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_XLAT, rlm_sql_config_t, simul_verify_query), NULL },
 #endif
        { "safe-characters", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_DEPRECATED, rlm_sql_config_t, allowed_chars), NULL },
        { "safe_characters", FR_CONF_OFFSET(PW_TYPE_STRING, rlm_sql_config_t, allowed_chars), "@abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789.-_: /" },
@@ -123,8 +119,7 @@ static const CONF_PARSER module_config[] = {
        { "accounting", FR_CONF_POINTER(PW_TYPE_SUBSECTION, NULL), (void const *) acct_config },
 
        { "post-auth", FR_CONF_POINTER(PW_TYPE_SUBSECTION, NULL), (void const *) postauth_config },
-
-       {NULL, -1, 0, NULL, NULL}
+       CONF_PARSER_TERMINATOR
 };
 
 /*
@@ -133,7 +128,7 @@ static const CONF_PARSER module_config[] = {
 static sql_fall_through_t fall_through(VALUE_PAIR *vp)
 {
        VALUE_PAIR *tmp;
-       tmp = pairfind(vp, PW_FALL_THROUGH, 0, TAG_ANY);
+       tmp = fr_pair_find_by_num(vp, PW_FALL_THROUGH, 0, TAG_ANY);
 
        return tmp ? tmp->vp_integer : FALL_THROUGH_DEFAULT;
 }
@@ -153,11 +148,12 @@ static size_t sql_escape_func(REQUEST *, char *out, size_t outlen, char const *i
  */
 static ssize_t sql_xlat(void *instance, REQUEST *request, char const *query, char *out, size_t freespace)
 {
-       rlm_sql_handle_t *handle = NULL;
-       rlm_sql_row_t row;
-       rlm_sql_t *inst = instance;
-       ssize_t ret = 0;
-       size_t len = 0;
+       rlm_sql_handle_t        *handle = NULL;
+       rlm_sql_row_t           row;
+       rlm_sql_t               *inst = instance;
+       sql_rcode_t             rcode;
+       ssize_t                 ret = 0;
+       size_t                  len = 0;
 
        /*
         *      Add SQL-User-Name attribute just in case it is needed
@@ -166,10 +162,8 @@ static ssize_t sql_xlat(void *instance, REQUEST *request, char const *query, cha
         */
        sql_set_user(inst, request, NULL);
 
-       handle = fr_connection_get(inst->pool);
-       if (!handle) {
-               return 0;
-       }
+       handle = fr_connection_get(inst->pool); /* connection pool should produce error */
+       if (!handle) return 0;
 
        rlm_sql_query_log(inst, request, NULL, query);
 
@@ -183,9 +177,10 @@ static ssize_t sql_xlat(void *instance, REQUEST *request, char const *query, cha
                int numaffected;
                char buffer[21]; /* 64bit max is 20 decimal chars + null byte */
 
-               if (rlm_sql_query(&handle, inst, query) != RLM_SQL_OK) {
-                       char const *error = (inst->module->sql_error)(handle, inst->config);
-                       REDEBUG("SQL query failed: %s", error);
+               rcode = rlm_sql_query(inst, request, &handle, query);
+               if (rcode != RLM_SQL_OK) {
+               query_error:
+                       RERROR("SQL query failed: %s", fr_int2str(sql_rcode_table, rcode, "<INVALID>"));
 
                        ret = -1;
                        goto finish;
@@ -194,6 +189,7 @@ static ssize_t sql_xlat(void *instance, REQUEST *request, char const *query, cha
                numaffected = (inst->module->sql_affected_rows)(handle, inst->config);
                if (numaffected < 1) {
                        RDEBUG("SQL query affected no rows");
+                       (inst->driver->sql_finish_query)(handle, inst->config);
 
                        goto finish;
                }
@@ -210,7 +206,7 @@ static ssize_t sql_xlat(void *instance, REQUEST *request, char const *query, cha
 
                len = strlen(buffer);
                if (len >= freespace){
-                       RDEBUG("rlm_sql (%s): Can't write result, insufficient string space", inst->config->xlat_name);
+                       RDEBUG("rlm_sql (%s): Can't write result, insufficient string space", inst->name);
 
                        (inst->module->sql_finish_query)(handle, inst->config);
 
@@ -226,19 +222,13 @@ static ssize_t sql_xlat(void *instance, REQUEST *request, char const *query, cha
                goto finish;
        } /* else it's a SELECT statement */
 
-       if (rlm_sql_select_query(&handle, inst, query) != RLM_SQL_OK) {
-               ret = -1;  /* error handled by rlm_sql_select_query */
+       rcode = rlm_sql_select_query(inst, request, &handle, query);
+       if (rcode != RLM_SQL_OK) goto query_error;
 
-               goto finish;
-       }
-
-       ret = rlm_sql_fetch_row(&handle, inst);
-       if (ret) {
-               REDEBUG("SQL query failed");
-               (inst->module->sql_finish_select_query)(handle, inst->config);
-               ret = -1;
-
-               goto finish;
+       rcode = rlm_sql_fetch_row(inst, request, &handle);
+       if (rcode) {
+               (inst->driver->sql_finish_select_query)(handle, inst->config);
+               goto query_error;
        }
 
        row = handle->row;
@@ -286,22 +276,28 @@ static int generate_sql_clients(rlm_sql_t *inst)
        RADCLIENT *c;
 
        DEBUG("rlm_sql (%s): Processing generate_sql_clients",
-             inst->config->xlat_name);
+             inst->name);
 
        DEBUG("rlm_sql (%s) in generate_sql_clients: query is %s",
-             inst->config->xlat_name, inst->config->client_query);
+             inst->name, inst->config->client_query);
 
        handle = fr_connection_get(inst->pool);
-       if (!handle) {
-               return -1;
-       }
+       if (!handle) return -1;
 
-       if (rlm_sql_select_query(&handle, inst, inst->config->client_query) != RLM_SQL_OK) return -1;
+       if (rlm_sql_select_query(inst, NULL, &handle, inst->config->client_query) != RLM_SQL_OK) return -1;
 
-       while ((rlm_sql_fetch_row(&handle, inst) == 0) && (row = handle->row)) {
+       while ((rlm_sql_fetch_row(inst, NULL, &handle) == 0) && (row = handle->row)) {
+               int num_rows;
                char *server = NULL;
+
                i++;
 
+               num_rows = (inst->module->sql_num_fields)(handle, inst->config);
+               if (num_rows < 5) {
+                       WARN("SELECT returned too few rows.  Please do not edit 'client_query'");
+                       continue;
+               }
+
                /*
                 *  The return data for each row MUST be in the following order:
                 *
@@ -313,28 +309,28 @@ static int generate_sql_clients(rlm_sql_t *inst)
                 *  5. Virtual Server (optional)
                 */
                if (!row[0]){
-                       ERROR("rlm_sql (%s): No row id found on pass %d",inst->config->xlat_name,i);
+                       ERROR("rlm_sql (%s): No row id found on pass %d",inst->name,i);
                        continue;
                }
                if (!row[1]){
-                       ERROR("rlm_sql (%s): No nasname found for row %s",inst->config->xlat_name,row[0]);
+                       ERROR("rlm_sql (%s): No nasname found for row %s",inst->name,row[0]);
                        continue;
                }
                if (!row[2]){
-                       ERROR("rlm_sql (%s): No short name found for row %s",inst->config->xlat_name,row[0]);
+                       ERROR("rlm_sql (%s): No short name found for row %s",inst->name,row[0]);
                        continue;
                }
                if (!row[4]){
-                       ERROR("rlm_sql (%s): No secret found for row %s",inst->config->xlat_name,row[0]);
+                       ERROR("rlm_sql (%s): No secret found for row %s",inst->name,row[0]);
                        continue;
                }
 
-               if (((inst->module->sql_num_fields)(handle, inst->config) > 5) && (row[5] != NULL) && *row[5]) {
+               if ((num_rows > 5) && (row[5] != NULL) && *row[5]) {
                        server = row[5];
                }
 
                DEBUG("rlm_sql (%s): Adding client %s (%s) to %s clients list",
-                     inst->config->xlat_name,
+                     inst->name,
                      row[1], row[2], server ? server : "global");
 
                /* FIXME: We should really pass a proper ctx */
@@ -357,7 +353,7 @@ static int generate_sql_clients(rlm_sql_t *inst)
                }
 
                DEBUG("rlm_sql (%s): Client \"%s\" (%s) added", c->longname, c->shortname,
-                     inst->config->xlat_name);
+                     inst->name);
        }
 
        (inst->module->sql_finish_select_query)(handle, inst->config);
@@ -382,7 +378,7 @@ static size_t sql_escape_func(UNUSED REQUEST *request, char *out, size_t outlen,
                /*
                 *      Allow all multi-byte UTF8 characters.
                 */
-               utf8_len = fr_utf8_char((uint8_t const *) in);
+               utf8_len = fr_utf8_char((uint8_t const *) in, -1);
                if (utf8_len > 1) {
                        if (outlen <= utf8_len) break;
 
@@ -403,7 +399,7 @@ static size_t sql_escape_func(UNUSED REQUEST *request, char *out, size_t outlen,
                switch (in[0]) {
                case '\n':
                        if (outlen <= 2) break;
-                       out[0] = '\'';
+                       out[0] = '\\';
                        out[1] = 'n';
 
                        in++;
@@ -414,7 +410,7 @@ static size_t sql_escape_func(UNUSED REQUEST *request, char *out, size_t outlen,
 
                case '\r':
                        if (outlen <= 2) break;
-                       out[0] = '\'';
+                       out[0] = '\\';
                        out[1] = 'r';
 
                        in++;
@@ -425,7 +421,7 @@ static size_t sql_escape_func(UNUSED REQUEST *request, char *out, size_t outlen,
 
                case '\t':
                        if (outlen <= 2) break;
-                       out[0] = '\'';
+                       out[0] = '\\';
                        out[1] = 't';
 
                        in++;
@@ -506,16 +502,21 @@ int sql_set_user(rlm_sql_t *inst, REQUEST *request, char const *username)
                return -1;
        }
 
-       vp = pairalloc(request->packet, inst->sql_user);
+       vp = fr_pair_afrom_da(request->packet, inst->sql_user);
        if (!vp) {
                talloc_free(expanded);
                return -1;
        }
 
-       pairstrsteal(vp, expanded);
+       fr_pair_value_strsteal(vp, expanded);
        RDEBUG2("SQL-User-Name set to '%s'", vp->vp_strvalue);
        vp->op = T_OP_SET;
-       radius_pairmove(request, &request->packet->vps, vp, false);     /* needs to be pair move else op is not respected */
+
+       /*
+        *      Delete any existing SQL-User-Name, and replace it with ours.
+        */
+       fr_pair_delete_by_num(&request->packet->vps, vp->da->attr, vp->da->vendor, TAG_ANY);
+       fr_pair_add(&request->packet->vps, vp);
 
        return 0;
 }
@@ -523,7 +524,7 @@ int sql_set_user(rlm_sql_t *inst, REQUEST *request, char const *username)
 /*
  *     Do a set/unset user, so it's a bit clearer what's going on.
  */
-#define sql_unset_user(_i, _r) pairdelete(&_r->packet->vps, _i->sql_user->attr, _i->sql_user->vendor, TAG_ANY)
+#define sql_unset_user(_i, _r) fr_pair_delete_by_num(&_r->packet->vps, _i->sql_user->attr, _i->sql_user->vendor, TAG_ANY)
 
 static int sql_get_grouplist(rlm_sql_t *inst, rlm_sql_handle_t **handle, REQUEST *request,
                             rlm_sql_grouplist_t **phead)
@@ -542,11 +543,11 @@ static int sql_get_grouplist(rlm_sql_t *inst, rlm_sql_handle_t **handle, REQUEST
 
        if (radius_axlat(&expanded, request, inst->config->groupmemb_query, sql_escape_func, inst) < 0) return -1;
 
-       ret = rlm_sql_select_query(handle, inst, expanded);
+       ret = rlm_sql_select_query(inst, request, handle, expanded);
        talloc_free(expanded);
        if (ret != RLM_SQL_OK) return -1;
 
-       while (rlm_sql_fetch_row(handle, inst) == 0) {
+       while (rlm_sql_fetch_row(inst, request, handle) == 0) {
                row = (*handle)->row;
                if (!row)
                        break;
@@ -583,15 +584,26 @@ static int sql_get_grouplist(rlm_sql_t *inst, rlm_sql_handle_t **handle, REQUEST
  * The group membership query should only return one element which is the username. The returned
  * username will then be checked with the passed check string.
  */
+static int sql_groupcmp(void *instance, REQUEST *request, UNUSED VALUE_PAIR *request_vp,
+                       VALUE_PAIR *check, UNUSED VALUE_PAIR *check_pairs,
+                       UNUSED VALUE_PAIR **reply_pairs) CC_HINT(nonnull (1, 2, 4));
 
-static int CC_HINT(nonnull (1, 2, 4)) sql_groupcmp(void *instance, REQUEST *request, UNUSED VALUE_PAIR *request_vp,
-                                                  VALUE_PAIR *check, UNUSED VALUE_PAIR *check_pairs,
-                                                  UNUSED VALUE_PAIR **reply_pairs)
+static int sql_groupcmp(void *instance, REQUEST *request, UNUSED VALUE_PAIR *request_vp,
+                       VALUE_PAIR *check, UNUSED VALUE_PAIR *check_pairs,
+                       UNUSED VALUE_PAIR **reply_pairs)
 {
        rlm_sql_handle_t *handle;
        rlm_sql_t *inst = instance;
        rlm_sql_grouplist_t *head, *entry;
 
+       /*
+        *      No group queries, don't do group comparisons.
+        */
+       if (!inst->config->groupmemb_query) {
+               RWARN("Cannot do group comparison when group_membership_query is not set");
+               return 1;
+       }
+
        RDEBUG("sql_groupcmp");
 
        if (check->vp_length == 0){
@@ -653,6 +665,19 @@ static rlm_rcode_t rlm_sql_process_groups(rlm_sql_t *inst, REQUEST *request, rlm
 
        rad_assert(request->packet != NULL);
 
+       if (!inst->config->groupmemb_query) {
+               RWARN("Cannot do check groups when group_membership_query is not set");
+
+       do_nothing:
+               *do_fall_through = FALL_THROUGH_DEFAULT;
+
+               /*
+                *      Didn't add group attributes or allocate
+                *      memory, so don't do anything else.
+                */
+               return RLM_MODULE_NOTFOUND;
+       }
+
        /*
         *      Get the list of groups this user is a member of
         */
@@ -664,10 +689,7 @@ static rlm_rcode_t rlm_sql_process_groups(rlm_sql_t *inst, REQUEST *request, rlm
        }
        if (rows == 0) {
                RDEBUG2("User not found in any groups");
-               rcode = RLM_MODULE_NOTFOUND;
-               *do_fall_through = FALL_THROUGH_DEFAULT;
-
-               goto finish;
+               goto do_nothing;
        }
        rad_assert(head);
 
@@ -677,9 +699,9 @@ static rlm_rcode_t rlm_sql_process_groups(rlm_sql_t *inst, REQUEST *request, rlm
         *      Add the Sql-Group attribute to the request list so we know
         *      which group we're retrieving attributes for
         */
-       sql_group = pairmake_packet("Sql-Group", NULL, T_OP_EQ);
+       sql_group = pair_make_request(inst->group_da->name, NULL, T_OP_EQ);
        if (!sql_group) {
-               REDEBUG("Error creating Sql-Group attribute");
+               REDEBUG("Error creating %s attribute", inst->group_da->name);
                rcode = RLM_MODULE_FAIL;
                goto finish;
        }
@@ -688,7 +710,7 @@ static rlm_rcode_t rlm_sql_process_groups(rlm_sql_t *inst, REQUEST *request, rlm
        do {
        next:
                rad_assert(entry != NULL);
-               pairstrcpy(sql_group, entry->name);
+               fr_pair_value_strcpy(sql_group, entry->name);
 
                if (inst->config->authorize_group_check_query) {
                        vp_cursor_t cursor;
@@ -718,9 +740,11 @@ static rlm_rcode_t rlm_sql_process_groups(rlm_sql_t *inst, REQUEST *request, rlm
                         */
                        if ((rows > 0) &&
                            (paircompare(request, request->packet->vps, check_tmp, &request->reply->vps) != 0)) {
-                               pairfree(&check_tmp);
+                               fr_pair_list_free(&check_tmp);
                                entry = entry->next;
 
+                               if (!entry) break;
+
                                goto next;      /* != continue */
                        }
 
@@ -737,7 +761,7 @@ static rlm_rcode_t rlm_sql_process_groups(rlm_sql_t *inst, REQUEST *request, rlm
                                rdebug_pair(L_DBG_LVL_2, request, vp, NULL);
                        }
                        REXDENT();
-                       radius_pairmove(request, &request->config_items, check_tmp, true);
+                       radius_pairmove(request, &request->config, check_tmp, true);
                        check_tmp = NULL;
                }
 
@@ -782,7 +806,7 @@ static rlm_rcode_t rlm_sql_process_groups(rlm_sql_t *inst, REQUEST *request, rlm
 
 finish:
        talloc_free(head);
-       pairdelete(&request->packet->vps, PW_SQL_GROUP, 0, TAG_ANY);
+       fr_pair_delete_by_num(&request->packet->vps, inst->group_da->attr, 0, TAG_ANY);
 
        return rcode;
 }
@@ -792,7 +816,7 @@ static int mod_detach(void *instance)
 {
        rlm_sql_t *inst = instance;
 
-       if (inst->pool) fr_connection_pool_delete(inst->pool);
+       if (inst->pool) fr_connection_pool_free(inst->pool);
 
        /*
         *  We need to explicitly free all children, so if the driver
@@ -813,7 +837,7 @@ static int mod_detach(void *instance)
        return 0;
 }
 
-static int mod_instantiate(CONF_SECTION *conf, void *instance)
+static int mod_bootstrap(CONF_SECTION *conf, void *instance)
 {
        rlm_sql_t *inst = instance;
 
@@ -823,52 +847,82 @@ static int mod_instantiate(CONF_SECTION *conf, void *instance)
        inst->config = &inst->myconfig;
        inst->cs = conf;
 
-       inst->config->xlat_name = cf_section_name2(conf);
-       if (!inst->config->xlat_name) {
-               inst->config->xlat_name = cf_section_name1(conf);
-       } else {
-               char *group_name;
-               DICT_ATTR const *da;
-               ATTR_FLAGS flags;
+       inst->name = cf_section_name2(conf);
+       if (!inst->name) inst->name = cf_section_name1(conf);
 
-               /*
-                *      Allocate room for <instance>-SQL-Group
-                */
-               group_name = talloc_typed_asprintf(inst, "%s-SQL-Group", inst->config->xlat_name);
-               DEBUG("rlm_sql (%s): Creating new attribute %s",
-                     inst->config->xlat_name, group_name);
-
-               memset(&flags, 0, sizeof(flags));
-               if (dict_addattr(group_name, -1, 0, PW_TYPE_STRING, flags) < 0) {
-                       ERROR("rlm_sql (%s): Failed to create "
-                              "attribute %s: %s", inst->config->xlat_name, group_name,
-                              fr_strerror());
-                       return -1;
-               }
+       /*
+        *      Load the appropriate driver for our database.
+        *
+        *      We need this to check if the sql_fields callback is provided.
+        */
+       inst->handle = fr_dlopenext(inst->config->sql_driver_name);
+       if (!inst->handle) {
+               ERROR("Could not link driver %s: %s", inst->config->sql_driver_name, fr_strerror());
+               ERROR("Make sure it (and all its dependent libraries!) are in the search path of your system's ld");
+               return -1;
+       }
 
-               da = dict_attrbyname(group_name);
-               if (!da) {
-                       ERROR("rlm_sql (%s): Failed to create "
-                              "attribute %s", inst->config->xlat_name, group_name);
-                       return -1;
+       inst->module = (rlm_sql_module_t *) dlsym(inst->handle,  inst->config->sql_driver_name);
+       if (!inst->module) {
+               ERROR("Could not link symbol %s: %s", inst->config->sql_driver_name, dlerror());
+               return -1;
+       }
+
+       INFO("rlm_sql (%s): Driver %s (module %s) loaded and linked", inst->name,
+            inst->config->sql_driver_name, inst->module->name);
+
+       if (inst->config->groupmemb_query) {
+               if (cf_section_name2(conf)) {
+                       char buffer[256];
+
+                       snprintf(buffer, sizeof(buffer), "%s-SQL-Group", inst->name);
+
+                       if (paircompare_register_byname(buffer, dict_attrbyvalue(PW_USER_NAME, 0),
+                                                       false, sql_groupcmp, inst) < 0) {
+                               ERROR("Error registering group comparison: %s", fr_strerror());
+                               return -1;
+                       }
+
+                       inst->group_da = dict_attrbyname(buffer);
+
+                       /*
+                        *      We're the default instance
+                        */
+               } else {
+                       if (paircompare_register_byname("SQL-Group", dict_attrbyvalue(PW_USER_NAME, 0),
+                                                       false, sql_groupcmp, inst) < 0) {
+                               ERROR("Error registering group comparison: %s", fr_strerror());
+                               return -1;
+                       }
+
+                       inst->group_da = dict_attrbyname("SQL-Group");
                }
 
-               if (inst->config->groupmemb_query) {
-                       DEBUG("rlm_sql (%s): Registering sql_groupcmp for %s",
-                             inst->config->xlat_name, group_name);
-                       paircompare_register(da, dict_attrbyvalue(PW_USER_NAME, 0),
-                                            false, sql_groupcmp, inst);
+               if (!inst->group_da) {
+                       ERROR("Failed resolving group attribute");
+                       return -1;
                }
        }
 
-       rad_assert(inst->config->xlat_name);
+       /*
+        *      Register the SQL xlat function
+        */
+       xlat_register(inst->name, sql_xlat, sql_escape_func, inst);
+
+       return 0;
+}
+
+
+static int mod_instantiate(CONF_SECTION *conf, void *instance)
+{
+       rlm_sql_t *inst = instance;
 
        /*
         *      Complain if the strings exist, but are empty.
         */
 #define CHECK_STRING(_x) if (inst->config->_x && !inst->config->_x[0]) \
 do { \
-       WARN("rlm_sql (%s): " STRINGIFY(_x) " is empty.  Please delete it from the configuration", inst->config->xlat_name);\
+       WARN("rlm_sql (%s): " STRINGIFY(_x) " is empty.  Please delete it from the configuration", inst->name);\
        inst->config->_x = NULL;\
 } while (0)
 
@@ -879,14 +933,10 @@ do { \
        CHECK_STRING(authorize_group_reply_query);
        CHECK_STRING(simul_count_query);
        CHECK_STRING(simul_verify_query);
-       CHECK_STRING(open_query);
+       CHECK_STRING(connect_query);
        CHECK_STRING(client_query);
-
-       /*
-        *      Sanity check for crazy people.
-        */
        if (strncmp(inst->config->sql_driver_name, "rlm_sql_", 8) != 0) {
-               ERROR("rlm_sql (%s): \"%s\" is NOT an SQL driver!", inst->config->xlat_name, inst->config->sql_driver_name);
+               ERROR("rlm_sql (%s): \"%s\" is NOT an SQL driver!", inst->name, inst->config->sql_driver_name);
                return -1;
        }
 
@@ -900,26 +950,20 @@ do { \
        if (!inst->config->groupmemb_query) {
                if (inst->config->authorize_group_check_query) {
                        WARN("rlm_sql (%s): Ignoring authorize_group_reply_query as group_membership_query "
-                            "is not configured", inst->config->xlat_name);
+                            "is not configured", inst->name);
                }
 
                if (inst->config->authorize_group_reply_query) {
                        WARN("rlm_sql (%s): Ignoring authorize_group_check_query as group_membership_query "
-                            "is not configured", inst->config->xlat_name);
-               }
-       } else {
-               if (!inst->config->authorize_group_check_query) {
-                       ERROR("rlm_sql (%s): authorize_group_check_query must be configured as group_membership_query "
-                             "is configured", inst->config->xlat_name);
-                       return -1;
+                            "is not configured", inst->name);
                }
 
-               if (!inst->config->authorize_group_reply_query) {
-                       ERROR("rlm_sql (%s): authorize_group_reply_query must be configured as group_membership_query "
-                             "is configured", inst->config->xlat_name);
-                       return -1;
+               if (!inst->config->read_groups) {
+                       WARN("rlm_sql (%s): Ignoring read_groups as group_membership_query "
+                            "is not configured", inst->name);
+                       inst->config->read_groups = false;
                }
-       }
+       } /* allow the group check / reply queries to be NULL */
 
        /*
         *      This will always exist, as cf_section_parse_init()
@@ -952,28 +996,6 @@ do { \
        inst->sql_select_query          = rlm_sql_select_query;
        inst->sql_fetch_row             = rlm_sql_fetch_row;
 
-       /*
-        *      Register the SQL xlat function
-        */
-       xlat_register(inst->config->xlat_name, sql_xlat, sql_escape_func, inst);
-
-       /*
-        *      Load the appropriate driver for our database
-        */
-       inst->handle = lt_dlopenext(inst->config->sql_driver_name);
-       if (!inst->handle) {
-               ERROR("Could not link driver %s: %s", inst->config->sql_driver_name, dlerror());
-               ERROR("Make sure it (and all its dependent libraries!) are in the search path of your system's ld");
-               return -1;
-       }
-
-       inst->module = (rlm_sql_module_t *) dlsym(inst->handle,
-                                                 inst->config->sql_driver_name);
-       if (!inst->module) {
-               ERROR("Could not link symbol %s: %s", inst->config->sql_driver_name, dlerror());
-               return -1;
-       }
-
        if (inst->module->mod_instantiate) {
                CONF_SECTION *cs;
                char const *name;
@@ -1001,28 +1023,20 @@ do { \
                }
        }
 
-       inst->ef = exfile_init(inst, 64, 30);
+       inst->ef = exfile_init(inst, 256, 30, true);
        if (!inst->ef) {
                cf_log_err_cs(conf, "Failed creating log file context");
                return -1;
        }
 
-       INFO("rlm_sql (%s): Driver %s (module %s) loaded and linked", inst->config->xlat_name,
-            inst->config->sql_driver_name, inst->module->name);
-
        /*
         *      Initialise the connection pool for this instance
         */
-       INFO("rlm_sql (%s): Attempting to connect to database \"%s\"", inst->config->xlat_name, inst->config->sql_db);
+       INFO("rlm_sql (%s): Attempting to connect to database \"%s\"", inst->name, inst->config->sql_db);
 
        inst->pool = fr_connection_pool_module_init(inst->cs, inst, mod_conn_create, NULL, NULL);
        if (!inst->pool) return -1;
 
-       if (inst->config->groupmemb_query) {
-               paircompare_register(dict_attrbyvalue(PW_SQL_GROUP, 0),
-                               dict_attrbyvalue(PW_USER_NAME, 0), false, sql_groupcmp, inst);
-       }
-
        if (inst->config->do_clients) {
                if (generate_sql_clients(inst) == -1){
                        ERROR("Failed to load clients from SQL");
@@ -1033,8 +1047,8 @@ do { \
        return RLM_MODULE_OK;
 }
 
-
-static rlm_rcode_t CC_HINT(nonnull) mod_authorize(void *instance, REQUEST *request)
+static rlm_rcode_t mod_authorize(void *instance, REQUEST *request) CC_HINT(nonnull);
+static rlm_rcode_t mod_authorize(void *instance, REQUEST *request)
 {
        rlm_rcode_t rcode = RLM_MODULE_NOOP;
 
@@ -1099,7 +1113,7 @@ static rlm_rcode_t CC_HINT(nonnull) mod_authorize(void *instance, REQUEST *reque
                rows = sql_getvpdata(request, inst, request, &handle, &check_tmp, expanded);
                TALLOC_FREE(expanded);
                if (rows < 0) {
-                       REDEBUG("SQL query error getting check attributes");
+                       REDEBUG("Error getting check attributes");
                        rcode = RLM_MODULE_FAIL;
                        goto error;
                }
@@ -1112,7 +1126,7 @@ static rlm_rcode_t CC_HINT(nonnull) mod_authorize(void *instance, REQUEST *reque
                RDEBUG2("User found in radcheck table");
                user_found = true;
                if (paircompare(request, request->packet->vps, check_tmp, &request->reply->vps) != 0) {
-                       pairfree(&check_tmp);
+                       fr_pair_list_free(&check_tmp);
                        check_tmp = NULL;
                        goto skipreply;
                }
@@ -1127,7 +1141,7 @@ static rlm_rcode_t CC_HINT(nonnull) mod_authorize(void *instance, REQUEST *reque
                        rdebug_pair(2, request, vp, NULL);
                }
                REXDENT();
-               radius_pairmove(request, &request->config_items, check_tmp, true);
+               radius_pairmove(request, &request->config, check_tmp, true);
 
                rcode = RLM_MODULE_OK;
                check_tmp = NULL;
@@ -1216,7 +1230,7 @@ skipreply:
                 *  Check for a default_profile or for a User-Profile.
                 */
                RDEBUG3("... falling-through to profile processing");
-               user_profile = pairfind(request->config_items, PW_USER_PROFILE, 0, TAG_ANY);
+               user_profile = fr_pair_find_by_num(request->config, PW_USER_PROFILE, 0, TAG_ANY);
 
                char const *profile = user_profile ?
                                      user_profile->vp_strvalue :
@@ -1275,8 +1289,8 @@ release:
        return rcode;
 
 error:
-       pairfree(&check_tmp);
-       pairfree(&reply_tmp);
+       fr_pair_list_free(&check_tmp);
+       fr_pair_list_free(&reply_tmp);
        sql_unset_user(inst, request);
 
        fr_connection_release(inst->pool, handle);
@@ -1323,16 +1337,20 @@ static int acct_redundant(rlm_sql_t *inst, REQUEST *request, sql_acct_section_t
                goto finish;
        }
 
+       /*
+        *      If we can't find a matching config item we do
+        *      nothing so return RLM_MODULE_NOOP.
+        */
        item = cf_reference_item(NULL, section->cs, path);
        if (!item) {
-               rcode = RLM_MODULE_FAIL;
+               RWDEBUG("No such configuration item %s", path);
+               rcode = RLM_MODULE_NOOP;
 
                goto finish;
        }
-
        if (cf_item_is_section(item)){
-               REDEBUG("Sections are not supported as references");
-               rcode = RLM_MODULE_FAIL;
+               RWDEBUG("Sections are not supported as references");
+               rcode = RLM_MODULE_NOOP;
 
                goto finish;
        }
@@ -1376,41 +1394,57 @@ static int acct_redundant(rlm_sql_t *inst, REQUEST *request, sql_acct_section_t
 
                rlm_sql_query_log(inst, request, section, expanded);
 
+               sql_ret = rlm_sql_query(inst, request, &handle, expanded);
+               TALLOC_FREE(expanded);
+               RDEBUG("SQL query returned: %s", fr_int2str(sql_rcode_table, sql_ret, "<INVALID>"));
+
+               switch (sql_ret) {
+               /*
+                *  Query was a success! Now we just need to check if it did anything.
+                */
+               case RLM_SQL_OK:
+                       break;
+
+               /*
+                *  A general, unrecoverable server fault.
+                */
+               case RLM_SQL_ERROR:
                /*
-                *  If rlm_sql_query cannot use the socket it'll try and
-                *  reconnect. Reconnecting will automatically release
-                *  the current socket, and try to select a new one.
-                *
                 *  If we get RLM_SQL_RECONNECT it means all connections in the pool
                 *  were exhausted, and we couldn't create a new connection,
                 *  so we do not need to call fr_connection_release.
                 */
-               sql_ret = rlm_sql_query(&handle, inst, expanded);
-               TALLOC_FREE(expanded);
-               if (sql_ret == RLM_SQL_RECONNECT) {
+               case RLM_SQL_RECONNECT:
                        rcode = RLM_MODULE_FAIL;
                        goto finish;
-               }
-               rad_assert(handle);
 
                /*
-                *  Assume all other errors are incidental, and just meant our
-                *  operation failed and its not a client or SQL syntax error.
-                *
-                *  @fixme We should actually be able to distinguish between key
-                *  constraint violations (which we expect) and other errors.
+                *  Query was invalid, this is a terminal error, but we still need
+                *  to do cleanup, as the connection handle is still valid.
                 */
-               if (sql_ret == RLM_SQL_OK) {
-                       numaffected = (inst->module->sql_affected_rows)(handle, inst->config);
-                       if (numaffected > 0) {
-                               break;  /* A query succeeded, were done! */
-                       }
+               case RLM_SQL_QUERY_INVALID:
+                       rcode = RLM_MODULE_INVALID;
+                       goto finish;
 
-                       RDEBUG("No records updated");
+               /*
+                *  Driver found an error (like a unique key constraint violation)
+                *  that hinted it might be a good idea to try an alternative query.
+                */
+               case RLM_SQL_ALT_QUERY:
+                       goto next;
                }
+               rad_assert(handle);
 
+               /*
+                *  We need to have updated something for the query to have been
+                *  counted as successful.
+                */
+               numaffected = (inst->module->sql_affected_rows)(handle, inst->config);
                (inst->module->sql_finish_query)(handle, inst->config);
+               RDEBUG("%i record(s) updated", numaffected);
 
+               if (numaffected > 0) break;     /* A query succeeded, were done! */
+       next:
                /*
                 *  We assume all entries with the same name form a redundant
                 *  set of queries.
@@ -1427,7 +1461,6 @@ static int acct_redundant(rlm_sql_t *inst, REQUEST *request, sql_acct_section_t
                RDEBUG("Trying next query...");
        }
 
-       (inst->module->sql_finish_query)(handle, inst->config);
 
 finish:
        talloc_free(expanded);
@@ -1442,7 +1475,9 @@ finish:
 /*
  *     Accounting: Insert or update session data in our sql table
  */
-static rlm_rcode_t CC_HINT(nonnull) mod_accounting(void *instance, REQUEST * request) {
+static rlm_rcode_t mod_accounting(void *instance, REQUEST *request) CC_HINT(nonnull);
+static rlm_rcode_t mod_accounting(void *instance, REQUEST *request)
+{
        rlm_sql_t *inst = instance;
 
        if (inst->config->accounting.reference_cp) {
@@ -1463,8 +1498,9 @@ static rlm_rcode_t CC_HINT(nonnull) mod_accounting(void *instance, REQUEST * req
  *     max. number of logins, do a second pass and validate all
  *     logins by querying the terminal server (using eg. SNMP).
  */
-
-static rlm_rcode_t CC_HINT(nonnull) mod_checksimul(void *instance, REQUEST * request) {
+static rlm_rcode_t mod_checksimul(void *instance, REQUEST *request) CC_HINT(nonnull);
+static rlm_rcode_t mod_checksimul(void *instance, REQUEST * request)
+{
        rlm_rcode_t             rcode = RLM_MODULE_OK;
        rlm_sql_handle_t        *handle = NULL;
        rlm_sql_t               *inst = instance;
@@ -1481,17 +1517,17 @@ static rlm_rcode_t CC_HINT(nonnull) mod_checksimul(void *instance, REQUEST * req
 
        /* If simul_count_query is not defined, we don't do any checking */
        if (!inst->config->simul_count_query) {
+               RWDEBUG("Simultaneous-Use checking requires 'simul_count_query' to be configured");
                return RLM_MODULE_NOOP;
        }
 
-       if ((!request->username) || (request->username->vp_length == '\0')) {
+       if ((!request->username) || (request->username->vp_length == 0)) {
                REDEBUG("Zero Length username not permitted");
 
                return RLM_MODULE_INVALID;
        }
 
-
-       if(sql_set_user(inst, request, NULL) < 0) {
+       if (sql_set_user(inst, request, NULL) < 0) {
                return RLM_MODULE_FAIL;
        }
 
@@ -1508,12 +1544,12 @@ static rlm_rcode_t CC_HINT(nonnull) mod_checksimul(void *instance, REQUEST * req
                return RLM_MODULE_FAIL;
        }
 
-       if (rlm_sql_select_query(&handle, inst, expanded) != RLM_SQL_OK) {
+       if (rlm_sql_select_query(inst, request, &handle, expanded) != RLM_SQL_OK) {
                rcode = RLM_MODULE_FAIL;
-               goto finish;
+               goto release;   /* handle may no longer be valid */
        }
 
-       ret = rlm_sql_fetch_row(&handle, inst);
+       ret = rlm_sql_fetch_row(inst, request, &handle);
        if (ret != 0) {
                rcode = RLM_MODULE_FAIL;
                goto finish;
@@ -1551,27 +1587,37 @@ static rlm_rcode_t CC_HINT(nonnull) mod_checksimul(void *instance, REQUEST * req
                goto finish;
        }
 
-       if (rlm_sql_select_query(&handle, inst, expanded) != RLM_SQL_OK) goto finish;
+       if (rlm_sql_select_query(inst, request, &handle, expanded) != RLM_SQL_OK) goto release;
 
        /*
         *      Setup some stuff, like for MPP detection.
         */
        request->simul_count = 0;
 
-       if ((vp = pairfind(request->packet->vps, PW_FRAMED_IP_ADDRESS, 0, TAG_ANY)) != NULL) {
+       if ((vp = fr_pair_find_by_num(request->packet->vps, PW_FRAMED_IP_ADDRESS, 0, TAG_ANY)) != NULL) {
                ipno = vp->vp_ipaddr;
        }
 
-       if ((vp = pairfind(request->packet->vps, PW_CALLING_STATION_ID, 0, TAG_ANY)) != NULL) {
+       if ((vp = fr_pair_find_by_num(request->packet->vps, PW_CALLING_STATION_ID, 0, TAG_ANY)) != NULL) {
                call_num = vp->vp_strvalue;
        }
 
-       while (rlm_sql_fetch_row(&handle, inst) == 0) {
+       while (rlm_sql_fetch_row(inst, request, &handle) == 0) {
+               int num_rows;
+
                row = handle->row;
                if (!row) {
                        break;
                }
 
+               num_rows = (inst->module->sql_num_fields)(handle, inst->config);
+               if (num_rows < 8) {
+                       RDEBUG("Too few rows returned.  Please do not edit 'simul_verify_query'");
+                       rcode = RLM_MODULE_FAIL;
+
+                       goto finish;
+               }
+
                if (!row[2]){
                        RDEBUG("Cannot zap stale entry. No username present in entry");
                        rcode = RLM_MODULE_FAIL;
@@ -1599,7 +1645,7 @@ static rlm_rcode_t CC_HINT(nonnull) mod_checksimul(void *instance, REQUEST * req
                        /*
                         *      Stale record - zap it.
                         */
-                       if (inst->config->deletestalesessions == true) {
+                       if (inst->config->delete_stale_sessions == true) {
                                uint32_t framed_addr = 0;
                                char proto = 0;
                                int sess_time = 0;
@@ -1612,7 +1658,7 @@ static rlm_rcode_t CC_HINT(nonnull) mod_checksimul(void *instance, REQUEST * req
                                        else if (strcmp(row[7], "SLIP") == 0)
                                                proto = 'S';
                                }
-                               if (row[8])
+                               if ((num_rows > 8) && row[8])
                                        sess_time = atoi(row[8]);
                                session_zap(request, nas_addr, nas_port,
                                            row[2], row[1], framed_addr,
@@ -1645,9 +1691,9 @@ static rlm_rcode_t CC_HINT(nonnull) mod_checksimul(void *instance, REQUEST * req
                }
        }
 
-       finish:
-
+finish:
        (inst->module->sql_finish_select_query)(handle, inst->config);
+release:
        fr_connection_release(inst->pool, handle);
        talloc_free(expanded);
        sql_unset_user(inst, request);
@@ -1664,7 +1710,9 @@ static rlm_rcode_t CC_HINT(nonnull) mod_checksimul(void *instance, REQUEST * req
 /*
  *     Postauth: Write a record of the authentication attempt
  */
-static rlm_rcode_t CC_HINT(nonnull) mod_post_auth(void *instance, REQUEST * request) {
+static rlm_rcode_t mod_post_auth(void *instance, REQUEST *request) CC_HINT(nonnull);
+static rlm_rcode_t mod_post_auth(void *instance, REQUEST *request)
+{
        rlm_sql_t *inst = instance;
 
        if (inst->config->postauth.reference_cp) {
@@ -1682,29 +1730,22 @@ static rlm_rcode_t CC_HINT(nonnull) mod_post_auth(void *instance, REQUEST * requ
 /* globally exported name */
 extern module_t rlm_sql;
 module_t rlm_sql = {
-       RLM_MODULE_INIT,
-       "SQL",
-       RLM_TYPE_THREAD_SAFE,   /* type: reserved */
-       sizeof(rlm_sql_t),
-       module_config,
-       mod_instantiate,        /* instantiation */
-       mod_detach,             /* detach */
-       {
-               NULL,                   /* authentication */
-               mod_authorize,  /* authorization */
-               NULL,                   /* preaccounting */
+       .magic          = RLM_MODULE_INIT,
+       .name           = "sql",
+       .type           = RLM_TYPE_THREAD_SAFE,
+       .inst_size      = sizeof(rlm_sql_t),
+       .config         = module_config,
+       .bootstrap      = mod_bootstrap,
+       .instantiate    = mod_instantiate,
+       .detach         = mod_detach,
+       .methods = {
+               [MOD_AUTHORIZE]         = mod_authorize,
 #ifdef WITH_ACCOUNTING
-               mod_accounting, /* accounting */
-#else
-               NULL,
+               [MOD_ACCOUNTING]        = mod_accounting,
 #endif
 #ifdef WITH_SESSION_MGMT
-               mod_checksimul, /* checksimul */
-#else
-               NULL,
+               [MOD_SESSION]           = mod_checksimul,
 #endif
-               NULL,                   /* pre-proxy */
-               NULL,                   /* post-proxy */
-               mod_post_auth   /* post-auth */
+               [MOD_POST_AUTH]         = mod_post_auth
        },
 };