[sr-dev] git:master: acc: fix cdr extra2strar allocation issues

Module: sip-router Branch: master Commit: 295716363562d88bce3db39e0f1bc605b1cc8257 URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=2957163… Author: Pawel.Kuzak &lt;pawel.kuzak(a)1und1.de&gt; Committer: lucian balanceanu &lt;lucian.balanceanu(a)1and1.ro&gt; Date: Wed Jul 30 13:59:49 2014 +0300 acc: fix cdr extra2strar allocation issues - for more than 10 string cdr_extra parameters, the addresses used by the new parameters overwrite the previous ones; our solution is to allocate memory for the cdr extra params with pkg_malloc() and free it once it is no longer needed. --- modules/acc/acc.c | 7 ++++++- modules/acc/acc_cdr.c | 27 ++++++++++++++++++++------- modules/acc/acc_extra.c | 39 ++++++++++++++++++--------------------- modules/acc/acc_extra.h | 12 ++++++++++++ modules/acc/acc_mod.c | 3 --- 5 files changed, 56 insertions(+), 32 deletions(-) diff --git a/modules/acc/acc.c b/modules/acc/acc.c index 418e709..51dfde5 100644 --- a/modules/acc/acc.c +++ b/modules/acc/acc.c @@ -229,6 +229,7 @@ int acc_log_request( struct sip_msg *rq) char *p; int n; int m; + int o = 0; int i; struct tm *t; @@ -236,7 +237,9 @@ int acc_log_request( struct sip_msg *rq) m = core2strar( rq, val_arr, int_arr, type_arr); /* get extra values */ - m += extra2strar( log_extra, rq, val_arr+m, int_arr+m, type_arr+m); + o += extra2strar( log_extra, rq, val_arr+m, int_arr+m, type_arr+m); + + m += o; for ( i=0,p=log_msg ; i<m ; i++ ) { if (p+1+log_attrs[i].len+1+val_arr[i].len >= log_msg_end) { @@ -314,6 +317,8 @@ int acc_log_request( struct sip_msg *rq) acc_env.text.len, acc_env.text.s,(unsigned long)acc_env.ts, log_msg); } + /* free memory allocated by extra2strar */ + free_strar_mem( &(type_arr[m-o]), &(val_arr[m-o]), o, m); return 1; } diff --git a/modules/acc/acc_cdr.c b/modules/acc/acc_cdr.c index c8d2684..bc2e66b 100644 --- a/modules/acc/acc_cdr.c +++ b/modules/acc/acc_cdr.c @@ -131,6 +131,7 @@ static int db_write_cdr( struct dlg_cell* dialog, struct sip_msg* message) { int m = 0; + int n = 0; int i; db_func_t *df=NULL; db1_con_t *dh=NULL; @@ -163,11 +164,12 @@ static int db_write_cdr( struct dlg_cell* dialog, /* get extra values */ if (message) { - m += extra2strar( cdr_extra, + n += extra2strar( cdr_extra, message, cdr_value_array + m, cdr_int_array + m, cdr_type_array + m); + m += n; } else if (cdr_expired_dlg_enable){ LM_WARN( "fallback to dlg_only search because of message doesn't exist.\n"); m += extra2strar_dlg_only( cdr_extra, @@ -187,27 +189,34 @@ static int db_write_cdr( struct dlg_cell* dialog, if (df->use_table(dh, &acc_cdrs_table /*table*/) < 0) { LM_ERR("error in use_table\n"); - return -1; + goto error; } if(acc_db_insert_mode==1 && df->insert_delayed!=NULL) { if (df->insert_delayed(dh, db_cdr_keys, db_cdr_vals, m) < 0) { LM_ERR("failed to insert delayed into database\n"); - return -1; + goto error; } } else if(acc_db_insert_mode==2 && df->insert_async!=NULL) { if (df->insert_async(dh, db_cdr_keys, db_cdr_vals, m) < 0) { LM_ERR("failed to insert async into database\n"); - return -1; + goto error; } } else { if (df->insert(dh, db_cdr_keys, db_cdr_vals, m) < 0) { LM_ERR("failed to insert into database\n"); - return -1; + goto error; } } + /* Free memory allocated by acc_extra.c/extra2strar */ + free_strar_mem( &(cdr_type_array[m-n]), &(cdr_value_array[m-n]), n, m); return 0; + +error: + /* Free memory allocated by acc_extra.c/extra2strar */ + free_strar_mem( &(cdr_type_array[m-n]), &(cdr_value_array[m-n]), n, m); + return -1; } #endif @@ -221,6 +230,7 @@ static int log_write_cdr( struct dlg_cell* dialog, 2;// -2 because of the string ending '\n\0' char* message_position = NULL; int message_index = 0; + int extra_index = 0; int counter = 0; if(cdr_log_enable==0) @@ -235,7 +245,7 @@ static int log_write_cdr( struct dlg_cell* dialog, /* get extra values */ if (message) { - message_index += extra2strar( cdr_extra, + extra_index += extra2strar( cdr_extra, message, cdr_value_array + message_index, cdr_int_array + message_index, @@ -249,7 +259,7 @@ static int log_write_cdr( struct dlg_cell* dialog, cdr_type_array + message_index, &dlgb); } - + message_index += extra_index; for( counter = 0, message_position = cdr_message; counter < message_index ; @@ -296,6 +306,9 @@ static int log_write_cdr( struct dlg_cell* dialog, LM_GEN2( cdr_facility, log_level, "%s", cdr_message); + /* free memory allocated by extra2strar, nothing is done in case no extra strings were found by extra2strar */ + free_strar_mem( &(cdr_type_array[message_index-extra_index]), &(cdr_value_array[message_index-extra_index]), + extra_index, message_index); return 0; } diff --git a/modules/acc/acc_extra.c b/modules/acc/acc_extra.c index 4847109..e9b6e4d 100644 --- a/modules/acc/acc_extra.c +++ b/modules/acc/acc_extra.c @@ -63,16 +63,6 @@ /* here we copy the strings returned by int2str (which uses a static buffer) */ static char int_buf[INT2STR_MAX_LEN*MAX_ACC_INT_BUF]; -static char *static_detector = 0; - -void init_acc_extra(void) -{ - int i; - /* ugly trick to get the address of the static buffer */ - static_detector = int2str( (unsigned long)3, &i) + i; -} - - struct acc_extra *parse_acc_leg(char *extra_str) { struct acc_extra *legs; @@ -249,10 +239,10 @@ int extra2strar(struct acc_extra *extra, struct sip_msg *rq, str *val_arr, { pv_value_t value; int n; - int r; + int i; n = 0; - r = 0; + i = 0; while (extra) { /* get the value */ @@ -272,15 +262,22 @@ int extra2strar(struct acc_extra *extra, struct sip_msg *rq, str *val_arr, val_arr[n].len = 0; type_arr[n] = TYPE_NULL; } else { - /* set the value into the acc buffer */ - if (value.rs.s+value.rs.len==static_detector) { - val_arr[n].s = int_buf + r*INT2STR_MAX_LEN; - val_arr[n].len = value.rs.len; - memcpy(val_arr[n].s, value.rs.s, value.rs.len); - r++; - } else { - val_arr[n] = value.rs; - } + val_arr[n].s = (char *)pkg_malloc(value.rs.len); + if (val_arr[n].s == NULL ) { + LM_ERR("extra2strar: out of memory.\n"); + /* Cleanup already allocated memory and + return that we didn't do anything */ + for (i = 0; i < n ; i++) { + if (NULL != val_arr[i].s){ + pkg_free(val_arr[i].s); + val_arr[i].s = NULL; + } + } + n = 0; + goto done; + } + memcpy(val_arr[n].s, value.rs.s, value.rs.len); + val_arr[n].len = value.rs.len; if (value.flags&PV_VAL_INT) { int_arr[n] = value.ri; type_arr[n] = TYPE_INT; diff --git a/modules/acc/acc_extra.h b/modules/acc/acc_extra.h index 13112b9..c906683 100644 --- a/modules/acc/acc_extra.h +++ b/modules/acc/acc_extra.h @@ -43,6 +43,7 @@ #ifndef _ACC_EXTRA_H_ #define _ACC_EXTRA_H_ +#include "acc_api.h" #include "../../str.h" #include "../../pvar.h" #include "../../parser/msg_parser.h" @@ -72,5 +73,16 @@ int extra2int( struct acc_extra *extra, int *attrs ); int extra2attrs( struct acc_extra *extra, struct attr *attrs, int offset); #endif +static inline void free_strar_mem( char* type_arr, str* alloc_arr, int dim_arr, int dim_ext){ + int i = 0; + for ( i = 0; i < dim_arr; i ++ ) { + if (( TYPE_NULL != type_arr[i] ) && ( NULL != alloc_arr[i].s)) { + LM_DBG("Freeing memory, type is %d, message_index %d, index i %d\n", + type_arr[i], dim_ext - dim_arr, i); + pkg_free( alloc_arr[i].s) ; + alloc_arr[i].s = NULL; + } + } +} #endif diff --git a/modules/acc/acc_mod.c b/modules/acc/acc_mod.c index acd620c..831a137 100644 --- a/modules/acc/acc_mod.c +++ b/modules/acc/acc_mod.c @@ -528,9 +528,6 @@ static int mod_init( void ) return -1; } - /* init the extra engine */ - init_acc_extra(); - /* configure multi-leg accounting */ if (leg_info_str && (leg_info=parse_acc_leg(leg_info_str))==0 ) { LM_ERR("failed to parse multileg_info param\n");