@henningw requested changes on this pull request.

Thank you for the PR. I've did longer review today and added several comments.

Many comments are rather small, like removing some commented out code or review and adapt the log levels (which probably are set higher from development). The documentation was already mentioned from Carsten. I've added also some suggestions for naming of parameter.

There is probably one condition regarding memory allocation which needs to be fixed.

The major issue is that you use the DB API incorrect. You should not use raw queries to execute statements on the database. Instead the appropriate select and delete functions should be used. I've added some notes also with pointers to examples.

Just let me know if there are any questions.

@@ -171,6 +171,17 @@ modparam("ims_usrloc_pcscf", "db_url",

reflected in the database. This is slow but very reliable. This mode will ensure that no registration data is lost as a result of a restart or crash. + * 3 - DB_ONLY Scheme. All changes to usrloc are immediately

As mentioned, this should be done in the XML files in the doc sub-directory.

@@ -110,8 +116,10 @@ static param_export_t params[] = {

{"timer_interval", INT_PARAM, &timer_interval }, {"db_mode", INT_PARAM, &db_mode },

- {"match_contact_host_port", INT_PARAM, &match_contact_host_port }, - {"expires_grace", INT_PARAM, &expires_grace }, + {"match_contact_host_port", INT_PARAM, &match_contact_host_port },

As mentioned, this two parameter should be also documented in the XML. If the parameter are for the DB cleanup job, its probably better to name them in this direction, e.g. db_cleanup_interval or similar.

What is the purpose of the expired_pcontacts_timeout, if there is already the expires_grace value?

•                           return;
  

+ } + } + else{ + if ((_c->expires - act_time) + expires_grace >= 0) { + return; + } + } + } + else{ + // not found in DB: better delete in cache also + update_stat(_c->slot->d->expired, 1); + mem_delete_pcontact(_c->slot->d, _c); + return; + } + // PRM1945 : PUBLISH already sent: delete pcontact and pua after time window 30 secs

Not sure what PRM1945 is, if its some internal bug tracker please remove it.

@@ -277,6 +277,17 @@ int new_pcontact(struct udomain* _d, str* _contact, struct pcontact_info* _ci, s

(*_c)->num_service_routes = _ci->num_service_routes; } } + // add the rx session id + if ((_ci->rx_regsession_id) && (_ci->rx_regsession_id->len > 0) && (_ci->rx_regsession_id->s)) { + (*_c)->rx_session_id.s = shm_malloc(_ci->rx_regsession_id->len); + if (!((*_c)->rx_session_id.s)) { + LM_ERR("no more shm mem\n"); + goto out_of_memory; + } + memcpy((*_c)->rx_session_id.s, _ci->rx_regsession_id->s, _ci->rx_regsession_id->len); + (*_c)->rx_session_id.len = _ci->rx_regsession_id->len; + } + //(*_c)->str_callback_registered = 0;

if not needed, remove

@@ -406,9 +408,11 @@ int update_pcontact(struct udomain* _d, struct pcontact_info* _ci, struct pconta

//TODO: update path, etc

- if (db_mode == WRITE_THROUGH && db_update_pcontact(_c) != 0) { - LM_ERR("Error updating record in DB"); - return -1; + if ((db_mode == WRITE_THROUGH) || (db_mode == DB_ONLY)){ + if (db_update_pcontact(_c) != 0) {

Minor comment, would be nice to combine the two if statements, similar as done below in several instances.

+struct tm *t; +char str_time[100]; + +str query_location, aor, expiry_date; +query_location.s = p_location; +query_location.len = strlen(query_location.s); + +if (use_location_pcscf_table("location") < 0) { + LM_ERR("failed to use table location"); + return 0; +} + +expired_50secs = time(0) - expires_grace - audit_expired_pcontacts_timeout; + +t = localtime(&expired_50secs);

use the existing function time2str(..) for the conversion, it has already the format and error checks

int db_update_pcontact(pcontact_t* _c) { str impus, service_routes;

db_val_t match_values[2]; - db_key_t match_keys[2] = { &aor_col, &received_port_col }; - db_op_t op[2]; - db_key_t update_keys[8] = { &expires_col, &reg_state_col, - &service_routes_col, &received_col, - &received_port_col, &received_proto_col, - &rx_session_id_col, &public_ids_col }; + //db_key_t match_keys[2] = { &aor_col, &received_port_col };

remove

@@ -146,8 +151,8 @@ int db_update_pcontact(pcontact_t* _c)

VAL_NULL(match_values + 1) = 0; VAL_INT(match_values + 1) = _c->received_port; - op[0]=OP_EQ; - op[1]=OP_EQ; + // op[0]=OP_EQ;

remove

@@ -195,17 +200,22 @@ int db_update_pcontact(pcontact_t* _c)

SET_PROPER_NULL_FLAG(impus, values, 7); SET_STR_VALUE(values + 7, impus);

- if((ul_dbf.update(ul_dbh, match_keys, op, match_values, update_keys,values, 2, 8)) !=0){ - LM_ERR("could not update database info\n"); - return -1; - } + //if((ul_dbf.update(ul_dbh, match_keys, op, match_values, update_keys,values, 2, 8)) !=0){

remove

- if (ul_dbf.affected_rows && ul_dbf.affected_rows(ul_dbh) == 0) { - LM_DBG("no existing rows for an update... doing insert\n"); - if (db_insert_pcontact(_c) != 0) { - LM_ERR("Failed to insert a pcontact on update\n"); - } - } + if (db_insert_pcontact(_c) != 0) { + LM_ERR("Failed to insert a pcontact on update\n"); + return -1; + } + +// if (ul_dbf.affected_rows && ul_dbf.affected_rows(ul_dbh) == 0) {

remove

@@ -341,7 +351,7 @@ int db_insert_pcontact(struct pcontact* _c)

return -1; }

- if (ul_dbf.insert(ul_dbh, keys, values, 16) < 0) { + if (ul_dbf.insert_update(ul_dbh, keys, values, 16) < 0) {

Why you are changing the module behaviour from INSERT to INSERT ON DUPLICATE KEY UPDATE?

• 			}
  

+ } else { + if (rpc->struct_add(ah, "d", "Expires", (int) (c->expires - t)) < 0) { + unlock_ulslot(dom, i); + rpc->fault(ctx, 500, "Internal error adding expire"); + return; + } + } + + if (rpc->struct_add(ah, "S", "Path", &c->path) < 0) { + unlock_ulslot(dom, i); + rpc->fault(ctx, 500, "Internal error creating path struct"); + return; + } + + // if (rpc->struct_add(ah, "{", "Service Routes", &sr) < 0) {

This can be kept, it was commented out before.

#ifndef _UL_CALLBACKS_H #define _UL_CALLBACKS_H

#include "../../core/dprint.h" +#include "../../core/parser/msg_parser.h" +#include "../../core/parser/contact/parse_contact.h" +#include "../../core/ut.h" +#include "../ims_usrloc_pcscf/usrloc.h" +#include "../../lib/ims/ims_getters.h" +// %KTACT%-END bugfix__PRM19/0000138_no_str_sent

Should be probably removed

•            LM_CRIT("null callback function\n");
  

+ return E_BUG; + } + + /* build a new callback structure */ + if ( types & PCSCF_CONTACT_UPDATE){ + if (!(cbp_registrar=(struct ul_callback*)shm_malloc(sizeof( struct ul_callback)))) { + LM_ERR("no more share mem\n"); + return E_OUT_OF_MEM; + } + cbp_registrar->callback = f; + } + else{ + if (!(cbp_qos=(struct ul_callback*)shm_malloc(sizeof( struct ul_callback)))) { + LM_ERR("no more share mem\n"); + return E_OUT_OF_MEM;

You probably need to free the previous allocated memory at cbp_registrar.

}

+expiry_date.s = str_time; +expiry_date.len = expiry_str_len; + +len = query_location.len + expiry_str_len + 1/*nul*/; +query_buffer.s = (char*) pkg_malloc(len); +if (!query_buffer.s) { + LM_ERR("mo more pkg mem\n"); + return -1; +} +query_buffer_len = len; +snprintf(query_buffer.s, query_buffer_len, p_location, expiry_date.len, expiry_date.s); +query_buffer.len = strlen(query_buffer.s);//len; + +LM_INFO("location QUERY IS [%.*s] and len is %d\n", query_buffer.len, query_buffer.s, query_buffer.len);

As this is probably executed every timer run, it should probably be DEBUG log level.

+query_buffer.s = (char*) pkg_malloc(len);

+if (!query_buffer.s) { + LM_ERR("mo more pkg mem\n"); + return -1; +} +query_buffer_len = len; +snprintf(query_buffer.s, query_buffer_len, p_location, expiry_date.len, expiry_date.s); +query_buffer.len = strlen(query_buffer.s);//len; + +LM_INFO("location QUERY IS [%.*s] and len is %d\n", query_buffer.len, query_buffer.s, query_buffer.len); +if (ul_dbf.raw_query(ul_dbh, &query_buffer, &location_rs) != 0) { + LM_ERR("Unable to query DB for expired pcontacts\n"); + ul_dbf.free_result(ul_dbh, location_rs); +} else { + if (RES_ROW_N(location_rs) == 0) { + LM_INFO("no expired pcontacts found in DB\n");

See above

• LM_ERR("Unable to query DB for expired pcontacts\n");

+ ul_dbf.free_result(ul_dbh, location_rs); +} else { + if (RES_ROW_N(location_rs) == 0) { + LM_INFO("no expired pcontacts found in DB\n"); + ul_dbf.free_result(ul_dbh, location_rs); + goto done; + } + + for(i = 0; i < RES_ROW_N(location_rs); i++) { + row = RES_ROWS(location_rs) + i; + + aor.s = (char*) VAL_STRING(ROW_VALUES(row) + 1); + + if (VAL_NULL(ROW_VALUES(row) + 1) || aor.s == 0 || aor.s[0] == 0) { + LM_CRIT("empty aor record in table %s...skipping\n", _d->name->s);

You should use CRITICAL log level only for really critical topics. This looks more that ERROR is fine.

•      if (VAL_NULL(ROW_VALUES(row) + 1) || aor.s == 0 || aor.s[0] == 0) {
  

+ LM_CRIT("empty aor record in table %s...skipping\n", _d->name->s); + continue; + } + aor.len = strlen(aor.s); + ci = dbrow2info(ROW_VALUES(row) + 1, &aor); + if (!ci) { + LM_WARN("Failed to get contact info from DB.... continuing...\n"); + continue; + } + ci->aor = aor; + ci->searchflag = SEARCH_NORMAL; + ci->reg_state = PCONTACT_ANY; + if (get_pcontact_from_cache(_d, ci, &c, 0) == 0){ + /* + if (&c->head->public_identity)

remove

+str query_buffer = { 0, 0 };

+int query_buffer_len = 0; + +struct tm *t; +char str_time[100]; + +str query_location, aor, expiry_date; +query_location.s = p_location; +query_location.len = strlen(query_location.s); + +if (use_location_pcscf_table("location") < 0) { + LM_ERR("failed to use table location"); + return 0; +} + +expired_50secs = time(0) - expires_grace - audit_expired_pcontacts_timeout;

Consider using a better variable name

- ul_dbf.free_result(_c, res); - return 0; + +int get_pcontact(udomain_t* _d, pcontact_info_t* contact_info, struct pcontact** _c, int reverse_search) { + + int ret = get_pcontact_from_cache(_d, contact_info, _c, reverse_search); + + if (ret && (db_mode == DB_ONLY)){ + // LM_INFO("contact not found in cache for contact_info->via_port [%d]\n", contact_info->via_port);

remove

•            return 0;
  

+ } + LM_INFO("Handling Result for query received\n"); + + for(i = 0; i < RES_ROW_N(res); i++) { + row = RES_ROWS(res) + i; + + aor.s = (char*) VAL_STRING(ROW_VALUES(row) + 1); + if (VAL_NULL(ROW_VALUES(row) + 1) || aor.s == 0 || aor.s[0] == 0) { + LM_CRIT("empty aor record in table %s...skipping\n", _d->name->s); + continue; + } + aor.len = strlen(aor.s); + + if ((_aor->len==0 && !_aor->s) && (VAL_NULL(ROW_VALUES(row) + 5) || VAL_NULL(ROW_VALUES(row) + 6))){ + LM_CRIT("empty received_host or received_port record in table %s...skipping\n", _d->name->s);

You should use CRITICAL log level only for really critical topics. This looks more that ERROR is fine.

•                    LM_DBG("host [%.*s] and port [%.*s] not found in table %.*s\n", contact_info->received_host.len, contact_info->received_host.s, port.len, port.s, _d->name->len, _d->name->s);
  

+ } + + ul_dbf.free_result(db_handle, res); + if (query_buffer.s) + pkg_free(query_buffer.s); + return 0; + } + LM_INFO("Handling Result for query received\n"); + + for(i = 0; i < RES_ROW_N(res); i++) { + row = RES_ROWS(res) + i; + + aor.s = (char*) VAL_STRING(ROW_VALUES(row) + 1); + if (VAL_NULL(ROW_VALUES(row) + 1) || aor.s == 0 || aor.s[0] == 0) { + LM_CRIT("empty aor record in table %s...skipping\n", _d->name->s);

See below

+ len = query_location.len + querylen + 2; + query_buffer.s = (char*) pkg_malloc(len); + if (!query_buffer.s) { + LM_ERR("no more pkg mem\n"); + return 0; + } + + if (_aor->len>0 && _aor->s){ + snprintf(query_buffer.s, len, p_location, _aor->len, _aor->s); + } else { + snprintf(query_buffer.s, len, p_location, contact_info->received_host.len, contact_info->received_host.s, port.len, port.s); + } + query_buffer.len = strlen(query_buffer.s);//len; + + LM_INFO("QUERY IS [%.*s] and len is %d\n", query_buffer.len, query_buffer.s, query_buffer.len);

This should be probably DEBUG log level

+ if (_aor->len>0 && _aor->s){ + LM_INFO("Querying database for P-CSCF contact [%.*s]\n", _aor->len, _aor->s); + + p_location = "SELECT domain,aor,host,port,protocol,received,received_port,received_proto,rx_session_id,reg_state,expires,socket,service_routes,public_ids,path from location WHERE aor="%.*s""; // AND reg_state=%d"; + querylen = _aor->len; + } else { + LM_INFO("Querying database for P-CSCF received_host [%.*s] and received_port [%d]\n", contact_info->received_host.len, contact_info->received_host.s, contact_info->received_port); + p_location = "SELECT domain,aor,host,port,protocol,received,received_port,received_proto,rx_session_id,reg_state,expires,socket,service_routes,public_ids,path from location WHERE received="%.*s" AND received_port="%.*s""; + port.s = int2str(contact_info->received_port, &port.len); + querylen = contact_info->received_host.len + port.len; + } + query_location.s = p_location; + query_location.len = strlen(query_location.s); + + LM_INFO("TEST: Query location [%.*s]\n", query_location.len, query_location.s);

This should be probably DEBUG log level

• columns[11] = &socket_col;

- columns[12] = &service_routes_col; - columns[13] = &public_ids_col; - columns[14] = &path_col; + int i, len, querylen; + str aor, query_location, query_buffer, port={0,0}; + db1_con_t* db_handle = 0; + char *p_location; + + if (_aor->len>0 && _aor->s){ + LM_INFO("Querying database for P-CSCF contact [%.*s]\n", _aor->len, _aor->s); + + p_location = "SELECT domain,aor,host,port,protocol,received,received_port,received_proto,rx_session_id,reg_state,expires,socket,service_routes,public_ids,path from location WHERE aor="%.*s""; // AND reg_state=%d"; + querylen = _aor->len; + } else { + LM_INFO("Querying database for P-CSCF received_host [%.*s] and received_port [%d]\n", contact_info->received_host.len, contact_info->received_host.s, contact_info->received_port);

See below

• columns[6] = &received_port_col;

- columns[7] = &received_proto_col; - columns[8] = &rx_session_id_col; - columns[9] = &reg_state_col; - columns[10] = &expires_col; - columns[11] = &socket_col; - columns[12] = &service_routes_col; - columns[13] = &public_ids_col; - columns[14] = &path_col; + int i, len, querylen; + str aor, query_location, query_buffer, port={0,0}; + db1_con_t* db_handle = 0; + char *p_location; + + if (_aor->len>0 && _aor->s){ + LM_INFO("Querying database for P-CSCF contact [%.*s]\n", _aor->len, _aor->s);

See below

+ for(i = 0; i < RES_ROW_N(res); i++) { + row = RES_ROWS(res) + i; + + aor.s = (char*) VAL_STRING(ROW_VALUES(row) + 1); + if (VAL_NULL(ROW_VALUES(row) + 1) || aor.s == 0 || aor.s[0] == 0) { + LM_CRIT("empty aor record in table %s...skipping\n", _d->name->s); + continue; + } + aor.len = strlen(aor.s); + + if ((_aor->len==0 && !_aor->s) && (VAL_NULL(ROW_VALUES(row) + 5) || VAL_NULL(ROW_VALUES(row) + 6))){ + LM_CRIT("empty received_host or received_port record in table %s...skipping\n", _d->name->s); + continue; + } + LM_INFO("Convert database values extracted with AOR.");

DEBUG log level

- ul_dbf.free_result(_c, res); - return 0; + +int get_pcontact(udomain_t* _d, pcontact_info_t* contact_info, struct pcontact** _c, int reverse_search) { + + int ret = get_pcontact_from_cache(_d, contact_info, _c, reverse_search); + + if (ret && (db_mode == DB_ONLY)){ + // LM_INFO("contact not found in cache for contact_info->via_port [%d]\n", contact_info->via_port); + LM_INFO("contact not found in cache for contact_info->received_port [%d]\n", contact_info->received_port);

DEBUG log level?

+int get_pcontact(udomain_t* _d, pcontact_info_t* contact_info, struct pcontact** _c, int reverse_search) {

+ + int ret = get_pcontact_from_cache(_d, contact_info, _c, reverse_search); + + if (ret && (db_mode == DB_ONLY)){ + // LM_INFO("contact not found in cache for contact_info->via_port [%d]\n", contact_info->via_port); + LM_INFO("contact not found in cache for contact_info->received_port [%d]\n", contact_info->received_port); + // following if: change for CATT test tool - PCSCF redundancy test cases + if (contact_info->searchflag == SEARCH_RECEIVED){ + LM_INFO("Trying contact_info.extra_search_criteria = 0\n"); + contact_info->extra_search_criteria = 0; + ret = get_pcontact_from_cache(_d, contact_info, _c, reverse_search); + if (ret == 0){ + return ret; + } + LM_INFO("contact not found in cache for contact_info->via_port [%d]\n", contact_info->via_port);

see above

• if (ret && (db_mode == DB_ONLY)){

+ // LM_INFO("contact not found in cache for contact_info->via_port [%d]\n", contact_info->via_port); + LM_INFO("contact not found in cache for contact_info->received_port [%d]\n", contact_info->received_port); + // following if: change for CATT test tool - PCSCF redundancy test cases + if (contact_info->searchflag == SEARCH_RECEIVED){ + LM_INFO("Trying contact_info.extra_search_criteria = 0\n"); + contact_info->extra_search_criteria = 0; + ret = get_pcontact_from_cache(_d, contact_info, _c, reverse_search); + if (ret == 0){ + return ret; + } + LM_INFO("contact not found in cache for contact_info->via_port [%d]\n", contact_info->via_port); + contact_info->extra_search_criteria = SEARCH_SERVICE_ROUTES; + + contact_info->searchflag = SEARCH_NORMAL; + LM_INFO("Trying contact_info.searchflag = SEARCH_NORMAL\n");

see above

•                    contact_info->extra_search_criteria = 0;
  

+ ret = get_pcontact_from_cache(_d, contact_info, _c, reverse_search); + if (ret == 0){ + return ret; + } + LM_INFO("contact not found in cache for contact_info->via_port [%d]\n", contact_info->via_port); + contact_info->extra_search_criteria = SEARCH_SERVICE_ROUTES; + + contact_info->searchflag = SEARCH_NORMAL; + LM_INFO("Trying contact_info.searchflag = SEARCH_NORMAL\n"); + ret = get_pcontact_from_cache(_d, contact_info, _c, reverse_search); + if (ret == 0){ + return ret; + } + else { + LM_INFO("Trying contact_info.extra_search_criteria = 0\n");

see above

•                             return ret;
  

+ } + else { + LM_INFO("Trying contact_info.extra_search_criteria = 0\n"); + contact_info->extra_search_criteria = 0; + ret = get_pcontact_from_cache(_d, contact_info, _c, reverse_search); + if (ret == 0){ + return ret; + } + LM_INFO("contact not found in cache for contact_info->via_port [%d]\n", contact_info->via_port); + contact_info->extra_search_criteria = SEARCH_SERVICE_ROUTES; + contact_info->searchflag = SEARCH_RECEIVED; + } + } + else { + LM_INFO("Trying contact_info.extra_search_criteria = 0\n");

See above

•       }
  

+ else { + LM_INFO("Trying contact_info.extra_search_criteria = 0\n"); + contact_info->extra_search_criteria = 0; + ret = get_pcontact_from_cache(_d, contact_info, _c, reverse_search); + if (ret == 0){ + return ret; + } + LM_INFO("contact not found in cache for contact_info->via_port [%d]\n", contact_info->via_port); + contact_info->extra_search_criteria = SEARCH_SERVICE_ROUTES; + } + if (db_load_pcontact(_d, &contact_info->aor, 1/*insert_cache*/, _c, contact_info)){ + LM_INFO("loaded location from db for AOR [%.*s]\n", contact_info->aor.len, contact_info->aor.s); + return 0; + } else { + LM_INFO("download location DB failed for AOR [%.*s]\n", contact_info->aor.len, contact_info->aor.s);

Is this an ERROR?

• ul_dbf.free_result(_c, res);
  

- return 0; - } + if (!port.s) { + LM_DBG("aor [%.*s] not found in table %.*s\n",_aor->len, _aor->s, _d->name->len, _d->name->s); + } + else { + LM_DBG("host [%.*s] and port [%.*s] not found in table %.*s\n", contact_info->received_host.len, contact_info->received_host.s, port.len, port.s, _d->name->len, _d->name->s); + } + + ul_dbf.free_result(db_handle, res); + if (query_buffer.s) + pkg_free(query_buffer.s); + return 0; + } + LM_INFO("Handling Result for query received\n");

DEBUG

•    if (ul_dbf.use_table(db_handle, &_pua) < 0) {
  

+ LM_ERR("failed to use table pua"); + return 0; + } + + int len = query_pua.len + presentity_uri->len + 2; + query_buffer.s = (char*) pkg_malloc(len); + if (!query_buffer.s) { + LM_ERR("mo more pkg mem\n"); + return 0; + } + snprintf(query_buffer.s, len, p_pua, presentity_uri->len, presentity_uri->s); + query_buffer.len = strlen(query_buffer.s);//len; + + + LM_INFO("QUERY IS [%.*s] and len is %d\n", query_buffer.len, query_buffer.s, query_buffer.len);

DEBUG log level

•                           return;
  

+ } + } + } + else{ + // not found in DB: better delete in cache also + update_stat(_c->slot->d->expired, 1); + mem_delete_pcontact(_c->slot->d, _c); + return; + } + // PRM1945 : PUBLISH already sent: delete pcontact and pua after time window 30 secs + // currently not clear what happens if more contacts existing for one public_identity (pua) + // which are not expired + if ((_c->reg_state == PCONTACT_DEREG_PENDING_PUBLISH) && + ((_c->expires - act_time) + expires_grace + 30 <= 0)){ + LM_INFO("Deleting expired pcontact: <%.*s>\n", _c->aor.len, _c->aor.s);

DEBUG log level?

@@ -1077,91 +1113,408 @@ int preload_udomain(db1_con_t* _c, udomain_t* _d)

return -1; }

-pcontact_t* db_load_pcontact(db1_con_t* _c, udomain_t* _d, str *_aor) +int db_delete_presentityuri_from_pua(str *presentity_uri) +{ + db1_res_t* res = NULL; + str _pua; + + db1_con_t* db_handle = 0; + + char *p_pua = "DELETE from pua where pua.pres_uri ="%.*s"";

Do I understand it correctly that you delete data from the pua module table? I am afraid this is not the way it should be done. If you need this functionality, the proper way would be to use an API of the PUA module.

• columns[8] = &rx_session_id_col;

- columns[9] = &reg_state_col; - columns[10] = &expires_col; - columns[11] = &socket_col; - columns[12] = &service_routes_col; - columns[13] = &public_ids_col; - columns[14] = &path_col; + int i, len, querylen; + str aor, query_location, query_buffer, port={0,0}; + db1_con_t* db_handle = 0; + char *p_location; + + if (_aor->len>0 && _aor->s){ + LM_INFO("Querying database for P-CSCF contact [%.*s]\n", _aor->len, _aor->s); + + p_location = "SELECT domain,aor,host,port,protocol,received,received_port,received_proto,rx_session_id,reg_state,expires,socket,service_routes,public_ids,path from location WHERE aor="%.*s""; // AND reg_state=%d";

This is not the way you should interact with the DB API. There are actually methods to build a select query. You can find some examples e.g. in the usrloc/dlist.c or in usrloc_db.c files. This way the module will work with all database drivers that implement the proper functions.

• columns[12] = &service_routes_col;

- columns[13] = &public_ids_col; - columns[14] = &path_col; + int i, len, querylen; + str aor, query_location, query_buffer, port={0,0}; + db1_con_t* db_handle = 0; + char *p_location; + + if (_aor->len>0 && _aor->s){ + LM_INFO("Querying database for P-CSCF contact [%.*s]\n", _aor->len, _aor->s); + + p_location = "SELECT domain,aor,host,port,protocol,received,received_port,received_proto,rx_session_id,reg_state,expires,socket,service_routes,public_ids,path from location WHERE aor="%.*s""; // AND reg_state=%d"; + querylen = _aor->len; + } else { + LM_INFO("Querying database for P-CSCF received_host [%.*s] and received_port [%d]\n", contact_info->received_host.len, contact_info->received_host.s, contact_info->received_port); + p_location = "SELECT domain,aor,host,port,protocol,received,received_port,received_proto,rx_session_id,reg_state,expires,socket,service_routes,public_ids,path from location WHERE received="%.*s" AND received_port="%.*s"";

See above

+ return ret; + +} + + +int audit_usrloc_expired_pcontacts(udomain_t* _d) { +db1_res_t* location_rs = NULL; + +pcontact_info_t *ci; +db_row_t *row; +int i; +pcontact_t* c; +time_t expired_50secs; + +char *p_location = "SELECT domain,aor,host,port,protocol,received,received_port,received_proto,rx_session_id,reg_state,expires,socket,service_routes,public_ids,path from location WHERE expires < "%.*s"";

This is not the way you should interact with the DB API. There are actually methods to build a select query. You can find some examples e.g. in the usrloc/dlist.c or usrloc_db files. This way the module will work with all database drivers that implement the proper functions.

@petermarianF commented on this pull request.

@@ -301,7 +307,7 @@ int db_insert_pcontact(struct pcontact* _c)

SET_PROPER_NULL_FLAG(_c->rinstance, values, LP_RINSTANCE_IDX); SET_PROPER_NULL_FLAG(_c->rx_session_id, values, LP_RX_SESSION_ID_IDX);

- VAL_DOUBLE(GET_FIELD_IDX(values, LP_REG_STATE_IDX)) = _c->reg_state; + VAL_INT(GET_FIELD_IDX(values, LP_REG_STATE_IDX)) = _c->reg_state;

The download also uses VAL_INT and not VAL_DOUBLE: src/modules/ims_usrloc_pcscf/udomain.c: static inline pcontact_info_t* dbrow2info( db_val_t *vals, str *contact) ..... ci.reg_state = VAL_INT(vals + 8);