Hello,
At 1&1 we have spotted an issue related to the cdr_extra parameters: for
more than 10 string cdr_extra parameters, the addresses used by the new
parameters overwrite the previous ones (this did not happen in 3.1, but
is reproducible since at least 3.3).
I attached a patch that implements a solution where we allocate memory
for the cdr extra params with pkg_malloc() and free it once it is no
longer needed.
Daniel, if there is no comment related to this solution, I will commit
the patch.
Thank you,
Lucian Balaceanu
>From b157ca5d4a0a64a38dfba89c37625d579025568a Mon Sep 17 00:00:00 2001
From: lucian balanceanu <[email protected]>
Date: Wed, 23 Jul 2014 08:17:49 +0300
Subject: [PATCH] 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 | 35 ++++++++++++++++++++++++++++-------
modules/acc/acc_extra.c | 36 +++++++++++++++---------------------
modules/acc/acc_extra.h | 11 +++++++++++
modules/acc/acc_mod.c | 3 ---
5 files changed, 60 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..dd716c6 100644
--- a/modules/acc/acc_cdr.c
+++ b/modules/acc/acc_cdr.c
@@ -89,6 +89,14 @@ extern str cdr_duration_str;
extern str acc_cdrs_table;
extern int cdr_log_enable;
+#define FREE_EXTRA_MEM ( type_arr, alloc_arr, dim_arr, dim_ext) \
+ for ( int i = 0; i < dim_arr; i ++ ) { \
+ if (type_arr[i] != TYPE_NULL ) { \
+ 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); \
+ } \
+ } \
+
/* write all basic information to buffers(e.g. start-time ...) */
static int cdr_core2strar( struct dlg_cell* dlg,
str* values,
@@ -131,6 +139,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 +172,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 +197,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 +238,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 +253,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 +267,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 +314,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..1b6c48d 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,19 @@ 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++) {
+ pkg_free(val_arr[i].s);
+ }
+ 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..2d95810 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,15 @@ 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_arr[i] != TYPE_NULL ) {
+ 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) ;
+ }
+ }
+}
#endif
diff --git a/modules/acc/acc_mod.c b/modules/acc/acc_mod.c
index 06f859a..2f046d1 100644
--- a/modules/acc/acc_mod.c
+++ b/modules/acc/acc_mod.c
@@ -538,9 +538,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");
--
1.7.4.1
_______________________________________________
sr-dev mailing list
[email protected]
http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
