[Asterisk-code-review] cdr/cdr adaptive odbc.c check is nulleable column on table. (asterisk[master])

[Matt Jordan]

Matt Jordan has posted comments on this change.

Change subject: cdr/cdr_adaptive_odbc.c check is nulleable column on table.
......................................................................


Patch Set 3: Code-Review-2

I couldn't really tell what is going here from the issue description or commit message. Looking at the issue, however, I think there is a problem in the CDR core that your patch is attempting to work around, which is not the correct solution to this problem.

Looking at what is attempting to be inserted into the table from your log:

[Apr 21 03:33:29] WARNING[30476]: cdr_adaptive_odbc.c:752 odbc_log: cdr_adaptive_odbc: Insert failed on 'asterisk:cdr'.  CDR failed: INSERT INTO cdr (calldate,clid,dst,dcontext,channel,lastapp,lastdata,duration,billsec,disposition,amaflags,uniqueid) VALUES ({ ts '2015-04-21 03:33:24' },'"" <>','700','parkedcalls','Console/dsp','Park','default:701',5,5,'ANSWERED',3,'1429598004.6')

At first glance, nothing about that appears to be incorrect: that is, the data looks correct. However, from the columns in your table, we can see that one is missing: 'src'.

       > Found calldate column with type 93 with len 19, octetlen 19, and numlen (0,0)
       > Found clid column with type 12 with len 80, octetlen 255, and numlen (0,0)
       > Found src column with type 12 with len 80, octetlen 255, and numlen (0,0)
       > Found dst column with type 12 with len 80, octetlen 255, and numlen (0,0)
       > Found dcontext column with type 12 with len 80, octetlen 255, and numlen (0,0)
       > Found channel column with type 12 with len 80, octetlen 255, and numlen (0,0)
       > Found dstchannel column with type 12 with len 80, octetlen 255, and numlen (0,0)
       > Found lastapp column with type 12 with len 80, octetlen 255, and numlen (0,0)
       > Found lastdata column with type 12 with len 80, octetlen 255, and numlen (0,0)
       > Found duration column with type 4 with len 10, octetlen 10, and numlen (0,10)
       > Found billsec column with type 4 with len 10, octetlen 10, and numlen (0,10)
       > Found disposition column with type 12 with len 45, octetlen 255, and numlen (0,0)
       > Found amaflags column with type 4 with len 10, octetlen 10, and numlen (0,10)
       > Found accountcode column with type 12 with len 20, octetlen 255, and numlen (0,0)
       > Found uniqueid column with type 12 with len 150, octetlen 255, and numlen (0,0)
       > Found userfield column with type 12 with len 255, octetlen 255, and numlen (0,0)

The fact is, 'src' should *not* be missing. The value of 'src' is supposed to be the Party A caller number. My guess is that the value is NULL on the channel, which causes the ast_copy_string in cdr.c to simply set the CDR object being posted to the backends to be NULL as well:

		/* Party A */
		ast_assert(party_a != NULL);
		ast_copy_string(cdr_copy->accountcode, party_a->accountcode, sizeof(cdr_copy->accountcode));
		cdr_copy->amaflags = party_a->amaflags;
		ast_copy_string(cdr_copy->channel, party_a->name, sizeof(cdr_copy->channel));
		ast_callerid_merge(cdr_copy->clid, sizeof(cdr_copy->clid), party_a->caller_name, party_a->caller_number, "");
		ast_copy_string(cdr_copy->src, party_a->caller_number, sizeof(cdr_copy->src));
		ast_copy_string(cdr_copy->uniqueid, party_a->uniqueid, sizeof(cdr_copy->uniqueid));
		ast_copy_string(cdr_copy->lastapp, it_cdr->appl, sizeof(cdr_copy->lastapp));
		ast_copy_string(cdr_copy->lastdata, it_cdr->data, sizeof(cdr_copy->lastdata));
		ast_copy_string(cdr_copy->dst, it_cdr->exten, sizeof(cdr_copy->dst));
		ast_copy_string(cdr_copy->dcontext, it_cdr->context, sizeof(cdr_copy->dcontext));

This same problem could occur in other CDR backends as well, so the correct fix is to make sure that cdr_copy->src is set to an empty string if party_a->caller_number is NULL.

It probably should be:

ast_copy_string(cdr->src, S_OR(party_a->caller_number, ""), sizeof(cdr->src));

Note this is what it used to be in 11:

	ast_copy_string(cdr->src, S_OR(num, ""), sizeof(cdr->src));

-- 
To view, visit https://gerrit.asterisk.org/191
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I05d7ce71254e74ff9a931ebe7b418d803b88bc54
Gerrit-PatchSet: 3
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Rodrigo Ramirez Norambuena <decipher.hk at gmail.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Rodrigo Ramirez Norambuena <decipher.hk at gmail.com>
Gerrit-HasComments: No