[Asterisk-code-review] cdr/cdr adaptive odbc.c: Set character for quoted identifiers. (asterisk[master])

[Kevin Harwell]

Kevin Harwell has posted comments on this change.

Change subject: cdr/cdr_adaptive_odbc.c: Set character for quoted identifiers.
......................................................................


Patch Set 4:

(2 comments)

It looks like when updating the review you overwrote Josh's changes to the commit message. The commit message will need to be updated again with his changes.

https://gerrit.asterisk.org/#/c/246/4/cdr/cdr_adaptive_odbc.c
File cdr/cdr_adaptive_odbc.c:

Line 105: 	char quoted_identifiers[3];
What configured values are you expecting/allowed? If the only expected values are a double quote or single quote could this just be declared as a char type? Or are you also allowing for an escape character?

Also, should the value be validated?


Line 415: 		if (ast_strlen_zero(tableptr->quoted_identifiers)){
        : 			ast_copy_string(table, tableptr->table, sizeof(table));
        : 			ast_copy_string(schema, tableptr->schema, sizeof(schema));
        : 		} else {
        : 			snprintf(table, sizeof(table), "%s%s%s",
        : 				tableptr->quoted_identifiers, tableptr->table, tableptr->quoted_identifiers);
        : 			snprintf(schema, sizeof(schema), "%s%s%s",
        : 				tableptr->quoted_identifiers, tableptr->schema, tableptr->quoted_identifiers);
        : 		}
If tableptr->schema is NULL then the behavior becomes undefined.

Just my opinion/suggestion, but I would just skip the intermediate copying of the table and schema and copy them directly into the sql string similar to how it was originally except of course with the added quotations if configured.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0b9a56b79ca13a727a803d88ed3b8643e37632b8
Gerrit-PatchSet: 4
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Rodrigo Ramirez Norambuena <decipher.hk at gmail.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-HasComments: Yes