mirror of
https://github.com/asterisk/asterisk.git
synced 2025-09-24 22:58:21 +00:00
Fix column duplication bug in module reload for cdr_pgsql.
Prior to this patch, attempts to reload cdr_pgsql.so would cause the column list to keep its current data and then add a second copy during the reload. This would cause attempts to log the CDR to the database to fail. This patch also cleans up some unnecessary null checks for ast_free and deals with a few potential locking problems. (closes issue ASTERISK-19216) Reported by: Jacek Konieczny Review: https://reviewboard.asterisk.org/r/1711/ ........ Merged revisions 354263 from http://svn.asterisk.org/svn/asterisk/branches/1.8 ........ Merged revisions 354270 from http://svn.asterisk.org/svn/asterisk/branches/10 git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@354275 65c4cc65-6c06-0410-ace0-fbb531ad65f3
This commit is contained in:
139
cdr/cdr_pgsql.c
139
cdr/cdr_pgsql.c
@@ -1,7 +1,7 @@
|
||||
/*
|
||||
* Asterisk -- An open source telephony toolkit.
|
||||
*
|
||||
* Copyright (C) 2003 - 2006
|
||||
* Copyright (C) 2003 - 2012
|
||||
*
|
||||
* Matthew D. Hardeman <mhardemn@papersoft.com>
|
||||
* Adapted from the MySQL CDR logger originally by James Sharp
|
||||
@@ -90,6 +90,7 @@ static AST_RWLIST_HEAD_STATIC(psql_columns, columns);
|
||||
ast_free(sql); \
|
||||
ast_free(sql2); \
|
||||
AST_RWLIST_UNLOCK(&psql_columns); \
|
||||
ast_mutex_unlock(&pgsql_lock); \
|
||||
return -1; \
|
||||
} \
|
||||
} \
|
||||
@@ -103,6 +104,7 @@ static AST_RWLIST_HEAD_STATIC(psql_columns, columns);
|
||||
ast_free(sql); \
|
||||
ast_free(sql2); \
|
||||
AST_RWLIST_UNLOCK(&psql_columns); \
|
||||
ast_mutex_unlock(&pgsql_lock); \
|
||||
return -1; \
|
||||
} \
|
||||
} \
|
||||
@@ -198,14 +200,10 @@ static int pgsql_log(struct ast_cdr *cdr)
|
||||
struct ast_str *sql = ast_str_create(maxsize), *sql2 = ast_str_create(maxsize2);
|
||||
char buf[257], escapebuf[513], *value;
|
||||
int first = 1;
|
||||
|
||||
|
||||
if (!sql || !sql2) {
|
||||
if (sql) {
|
||||
ast_free(sql);
|
||||
}
|
||||
if (sql2) {
|
||||
ast_free(sql2);
|
||||
}
|
||||
ast_free(sql);
|
||||
ast_free(sql2);
|
||||
return -1;
|
||||
}
|
||||
|
||||
@@ -336,9 +334,10 @@ static int pgsql_log(struct ast_cdr *cdr)
|
||||
}
|
||||
}
|
||||
first = 0;
|
||||
}
|
||||
AST_RWLIST_UNLOCK(&psql_columns);
|
||||
}
|
||||
|
||||
LENGTHEN_BUF1(ast_str_strlen(sql2) + 2);
|
||||
AST_RWLIST_UNLOCK(&psql_columns);
|
||||
ast_str_append(&sql, 0, ")%s)", ast_str_buffer(sql2));
|
||||
ast_verb(11, "[%s]\n", ast_str_buffer(sql));
|
||||
|
||||
@@ -414,46 +413,36 @@ static int pgsql_log(struct ast_cdr *cdr)
|
||||
return 0;
|
||||
}
|
||||
|
||||
static int unload_module(void)
|
||||
/* This function should be called without holding the pgsql_columns lock */
|
||||
static void empty_columns(void)
|
||||
{
|
||||
struct columns *current;
|
||||
|
||||
ast_cdr_unregister(name);
|
||||
ast_cli_unregister_multiple(cdr_pgsql_status_cli, ARRAY_LEN(cdr_pgsql_status_cli));
|
||||
|
||||
PQfinish(conn);
|
||||
|
||||
if (pghostname) {
|
||||
ast_free(pghostname);
|
||||
}
|
||||
if (pgdbname) {
|
||||
ast_free(pgdbname);
|
||||
}
|
||||
if (pgdbuser) {
|
||||
ast_free(pgdbuser);
|
||||
}
|
||||
if (pgpassword) {
|
||||
ast_free(pgpassword);
|
||||
}
|
||||
if (pgdbport) {
|
||||
ast_free(pgdbport);
|
||||
}
|
||||
if (table) {
|
||||
ast_free(table);
|
||||
}
|
||||
if (encoding) {
|
||||
ast_free(encoding);
|
||||
}
|
||||
if (tz) {
|
||||
ast_free(tz);
|
||||
}
|
||||
|
||||
AST_RWLIST_WRLOCK(&psql_columns);
|
||||
while ((current = AST_RWLIST_REMOVE_HEAD(&psql_columns, list))) {
|
||||
ast_free(current);
|
||||
}
|
||||
AST_RWLIST_UNLOCK(&psql_columns);
|
||||
|
||||
}
|
||||
|
||||
static int unload_module(void)
|
||||
{
|
||||
ast_cdr_unregister(name);
|
||||
ast_cli_unregister_multiple(cdr_pgsql_status_cli, ARRAY_LEN(cdr_pgsql_status_cli));
|
||||
|
||||
PQfinish(conn);
|
||||
|
||||
ast_free(pghostname);
|
||||
ast_free(pgdbname);
|
||||
ast_free(pgdbuser);
|
||||
ast_free(pgpassword);
|
||||
ast_free(pgdbport);
|
||||
ast_free(table);
|
||||
ast_free(encoding);
|
||||
ast_free(tz);
|
||||
|
||||
empty_columns();
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
||||
@@ -474,9 +463,14 @@ static int config_module(int reload)
|
||||
return 0;
|
||||
}
|
||||
|
||||
ast_mutex_lock(&pgsql_lock);
|
||||
|
||||
if (!(var = ast_variable_browse(cfg, "global"))) {
|
||||
ast_config_destroy(cfg);
|
||||
return 0;
|
||||
ast_mutex_unlock(&pgsql_lock);
|
||||
ast_log(LOG_NOTICE, "cdr_pgsql configuration contains no global section, skipping module %s.\n",
|
||||
reload ? "reload" : "load");
|
||||
return -1;
|
||||
}
|
||||
|
||||
if (!(tmp = ast_variable_retrieve(cfg, "global", "hostname"))) {
|
||||
@@ -484,11 +478,10 @@ static int config_module(int reload)
|
||||
tmp = ""; /* connect via UNIX-socket by default */
|
||||
}
|
||||
|
||||
if (pghostname) {
|
||||
ast_free(pghostname);
|
||||
}
|
||||
ast_free(pghostname);
|
||||
if (!(pghostname = ast_strdup(tmp))) {
|
||||
ast_config_destroy(cfg);
|
||||
ast_mutex_unlock(&pgsql_lock);
|
||||
return -1;
|
||||
}
|
||||
|
||||
@@ -497,11 +490,10 @@ static int config_module(int reload)
|
||||
tmp = "asteriskcdrdb";
|
||||
}
|
||||
|
||||
if (pgdbname) {
|
||||
ast_free(pgdbname);
|
||||
}
|
||||
ast_free(pgdbname);
|
||||
if (!(pgdbname = ast_strdup(tmp))) {
|
||||
ast_config_destroy(cfg);
|
||||
ast_mutex_unlock(&pgsql_lock);
|
||||
return -1;
|
||||
}
|
||||
|
||||
@@ -510,11 +502,10 @@ static int config_module(int reload)
|
||||
tmp = "asterisk";
|
||||
}
|
||||
|
||||
if (pgdbuser) {
|
||||
ast_free(pgdbuser);
|
||||
}
|
||||
ast_free(pgdbuser);
|
||||
if (!(pgdbuser = ast_strdup(tmp))) {
|
||||
ast_config_destroy(cfg);
|
||||
ast_mutex_unlock(&pgsql_lock);
|
||||
return -1;
|
||||
}
|
||||
|
||||
@@ -523,11 +514,10 @@ static int config_module(int reload)
|
||||
tmp = "";
|
||||
}
|
||||
|
||||
if (pgpassword) {
|
||||
ast_free(pgpassword);
|
||||
}
|
||||
ast_free(pgpassword);
|
||||
if (!(pgpassword = ast_strdup(tmp))) {
|
||||
ast_config_destroy(cfg);
|
||||
ast_mutex_unlock(&pgsql_lock);
|
||||
return -1;
|
||||
}
|
||||
|
||||
@@ -536,11 +526,10 @@ static int config_module(int reload)
|
||||
tmp = "5432";
|
||||
}
|
||||
|
||||
if (pgdbport) {
|
||||
ast_free(pgdbport);
|
||||
}
|
||||
ast_free(pgdbport);
|
||||
if (!(pgdbport = ast_strdup(tmp))) {
|
||||
ast_config_destroy(cfg);
|
||||
ast_mutex_unlock(&pgsql_lock);
|
||||
return -1;
|
||||
}
|
||||
|
||||
@@ -549,11 +538,10 @@ static int config_module(int reload)
|
||||
tmp = "cdr";
|
||||
}
|
||||
|
||||
if (table) {
|
||||
ast_free(table);
|
||||
}
|
||||
ast_free(table);
|
||||
if (!(table = ast_strdup(tmp))) {
|
||||
ast_config_destroy(cfg);
|
||||
ast_mutex_unlock(&pgsql_lock);
|
||||
return -1;
|
||||
}
|
||||
|
||||
@@ -562,11 +550,10 @@ static int config_module(int reload)
|
||||
tmp = "LATIN9";
|
||||
}
|
||||
|
||||
if (encoding) {
|
||||
ast_free(encoding);
|
||||
}
|
||||
ast_free(encoding);
|
||||
if (!(encoding = ast_strdup(tmp))) {
|
||||
ast_config_destroy(cfg);
|
||||
ast_mutex_unlock(&pgsql_lock);
|
||||
return -1;
|
||||
}
|
||||
|
||||
@@ -574,12 +561,12 @@ static int config_module(int reload)
|
||||
tmp = "";
|
||||
}
|
||||
|
||||
if (tz) {
|
||||
ast_free(tz);
|
||||
tz = NULL;
|
||||
}
|
||||
ast_free(tz);
|
||||
tz = NULL;
|
||||
|
||||
if (!ast_strlen_zero(tmp) && !(tz = ast_strdup(tmp))) {
|
||||
ast_config_destroy(cfg);
|
||||
ast_mutex_unlock(&pgsql_lock);
|
||||
return -1;
|
||||
}
|
||||
|
||||
@@ -667,6 +654,7 @@ static int config_module(int reload)
|
||||
ast_log(LOG_ERROR, "Failed to query database columns: %s\n", pgerror);
|
||||
PQclear(result);
|
||||
unload_module();
|
||||
ast_mutex_unlock(&pgsql_lock);
|
||||
return AST_MODULE_LOAD_DECLINE;
|
||||
}
|
||||
|
||||
@@ -675,9 +663,13 @@ static int config_module(int reload)
|
||||
ast_log(LOG_ERROR, "cdr_pgsql: Failed to query database columns. No columns found, does the table exist?\n");
|
||||
PQclear(result);
|
||||
unload_module();
|
||||
ast_mutex_unlock(&pgsql_lock);
|
||||
return AST_MODULE_LOAD_DECLINE;
|
||||
}
|
||||
|
||||
/* Clear out the columns list. */
|
||||
empty_columns();
|
||||
|
||||
for (i = 0; i < rows; i++) {
|
||||
fname = PQgetvalue(result, i, 0);
|
||||
ftype = PQgetvalue(result, i, 1);
|
||||
@@ -706,7 +698,9 @@ static int config_module(int reload)
|
||||
} else {
|
||||
cur->hasdefault = 0;
|
||||
}
|
||||
AST_RWLIST_WRLOCK(&psql_columns);
|
||||
AST_RWLIST_INSERT_TAIL(&psql_columns, cur, list);
|
||||
AST_RWLIST_UNLOCK(&psql_columns);
|
||||
}
|
||||
}
|
||||
PQclear(result);
|
||||
@@ -719,13 +713,18 @@ static int config_module(int reload)
|
||||
|
||||
ast_config_destroy(cfg);
|
||||
|
||||
return ast_cdr_register(name, ast_module_info->description, pgsql_log);
|
||||
ast_mutex_unlock(&pgsql_lock);
|
||||
return 0;
|
||||
}
|
||||
|
||||
static int load_module(void)
|
||||
{
|
||||
ast_cli_register_multiple(cdr_pgsql_status_cli, sizeof(cdr_pgsql_status_cli) / sizeof(struct ast_cli_entry));
|
||||
return config_module(0) ? AST_MODULE_LOAD_DECLINE : 0;
|
||||
if (config_module(0)) {
|
||||
return AST_MODULE_LOAD_DECLINE;
|
||||
}
|
||||
return ast_cdr_register(name, ast_module_info->description, pgsql_log)
|
||||
? AST_MODULE_LOAD_DECLINE : 0;
|
||||
}
|
||||
|
||||
static int reload(void)
|
||||
|
Reference in New Issue
Block a user