2 Feb
2010
2 Feb
'10
8:43 a.m.
I ran into a very interesting problem last night with 'acc' that took
me a few hours to solve, so I am posting my workaround for collective
benefit. It is probably not the cleanest and most appropriate
solution because I am not extensively familiar with the 'acc' code;
this is my first dive into it. Thus, I do not presume to submit an
actual patch.
The problem I had was that PVs read from the db_extra parameter that
had an internal type of integer (TYPE_INT) because of their lexical
scope were not being correctly evaluated, resulting in bizarre values
being put in the database.
I am using K 3.0.0 on 64-bit Xen guest running CentOS with
db_postgres. I did not check to see if this problem occurs with
db_mysql as I do not have MySQL anywhere nearby, but I don't believe
it is happening at the database layer anyhow. I also know this
problem was not present in 1.5.x, or several of my installations would
be non-functional; this must have something to do with use of new
'srdb1' stuff in the database side of 'acc', I hypothesise.
Anyway, the actual problem went like this. Let us say, for simplicity
of example, that I am using 'acc' with Postgres backing and have
something like this:
modparam("acc", "db_extra",
"account_id=$avp(s:account_id)")
...
modparam("acc", "db_flag", 1)
What was happening was that if I did this in a request route:
$avp(s:account_id) = 42;
...
setflag(1);
if(!t_relay())
sl_reply_error();
exit;
Very strange and occasionally bizarre values, mostly duplicates of
other values I have in db_extra, would be written to acc.account_id (a
column of type 'integer') - at any rate, certainly not 42.
I enabled SQL statement logging on my PostgreSQL database to ensure
that the data being inserted was literally not 42, rather than this
being some sort of database-layer type conversion / representation
problem. Sure enough, it was true, although I also did notice that
'acc' encloses all inserted values in single quotes to be treated as
string literals. But this is immaterial, because RDBMs automatically
cast such input:
INSERT INTO acc (..., account_id) VALUES(..., '12');
Now, I could fix the problem by simply assigning the AVP a string
literal instead:
$avp(s:account_id) = '42';
Now the data was inserted normally.
I also tested with user variables ($var(...)) and the same was true,
so the problem is not AVP-specific. Also, all other evaluative
contexts for PVs with these values work fine, such as printing them.
The problem does not seem to be with 'db_postgres', neither with 'pv',
nor with 'avpops' -- it is particular to 'acc'.
After some hours of investigation I found that what was happening is
that the type of the PV being referenced is not being correctly packed
into the various arrays populated by extra2strar() in
modules_k/acc/acc_extra.c, and thus was being misread when iterated in
acc_db_request() in modules_k/acc/acc.c. Specifically, this check was
inadequate:
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++;
LM_INFO("New int tenant: %s\n",
val_arr[n].s);
} else {
val_arr[n] = value.rs;
LM_INFO("New str tenant: %s\n",
value.rs);
}
In my debugging, all PV values were showing up as "New str tenant"
regardless of whether they were natively integers, and regardless of
what the PV spec type was actually set to. The type was set correctly
on the PV spec, so of course the integer value got stored correctly in
int_var[] below:
if (value.flags&PV_VAL_INT) {
int_arr[n] = value.ri;
type_arr[n] = TYPE_INT;
} else {
type_arr[n] = TYPE_STR;
}
... but since 'acc' wants to treat all extra values as strings, this
was not sufficient, because what was failing was this condition:
if (value.rs.s+value.rs.len==static_detector)
I guess value.rs.s in case of the PV value being PV_VAL_INT is not
null terminated properly? This I truly do not know.
In any event, I just added a check on the PV flags to see if it is an
integer value and this fixed my problem:
if (value.rs.s+value.rs.len==static_detector
|| value.flags & PV_VAL_INT) {
... however, I do not know that this was the most insightful solution.
This is rather critical I think, because it means anyone trying to
store PV values that are initialised with a natively numerical type in
script (or extracted as a numerical value from a database, e.g.
sql_query("SELECT account_id::integer FROM table ..."), I found this
to be a problem as well) cannot store them in the database reliably
using 'db_extra'.
Thanks!
--
Alex Balashov - Principal
Evariste Systems LLC
Tel : +1 678-954-0670
Direct : +1 678-954-0671
Web : http://www.evaristesys.com/