[Asterisk-code-review] res odbc: fix missing SQL error diagnostic (asterisk[master])

Kevin Harwell asteriskteam at digium.com
Fri Sep 28 10:39:22 CDT 2018


Kevin Harwell has submitted this change and it was merged. ( https://gerrit.asterisk.org/10285 )

Change subject: res_odbc: fix missing SQL error diagnostic
......................................................................

res_odbc: fix missing SQL error diagnostic

On SQL error there is not diagnostic information about this error.
There is only
WARNING res_odbc.c: SQL Execute error -1!

The function ast_odbc_print_errors calls a SQLGetDiagField to get the number
of available diagnostic records, but the SQLGetDiagField returns 0.
However SQLGetDiagRec could return one diagnostic records in this case.

Looking at many example of getting diagnostics error information
I found out that the best way it's to use only SQLGetDiagRec
while it returns SQL_SUCCESS.

Also this patch adds calls of ast_odbc_print_errors on SQL_ERROR
to res_config_odbc.

ASTERISK-28065 #close

Change-Id: Iba5ae5470ac49ecd911dd084effbe9efac68ccc1
---
M res/res_config_odbc.c
M res/res_odbc.c
2 files changed, 10 insertions(+), 7 deletions(-)

Approvals:
  George Joseph: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, but someone else must approve
  Kevin Harwell: Looks good to me, approved; Approved for Submit



diff --git a/res/res_config_odbc.c b/res/res_config_odbc.c
index be920d6..6eea305 100644
--- a/res/res_config_odbc.c
+++ b/res/res_config_odbc.c
@@ -116,6 +116,9 @@
 
 	res = SQLPrepare(stmt, (unsigned char *)cps->sql, SQL_NTS);
 	if ((res != SQL_SUCCESS) && (res != SQL_SUCCESS_WITH_INFO)) {
+		if (res == SQL_ERROR) {
+			ast_odbc_print_errors(SQL_HANDLE_STMT, stmt, "SQL Prepare");
+		}
 		ast_log(LOG_WARNING, "SQL Prepare failed! [%s]\n", cps->sql);
 		SQLFreeHandle (SQL_HANDLE_STMT, stmt);
 		return NULL;
@@ -631,6 +634,9 @@
 
 	res = SQLPrepare(stmt, (unsigned char *)ast_str_buffer(sql), SQL_NTS);
 	if ((res != SQL_SUCCESS) && (res != SQL_SUCCESS_WITH_INFO)) {
+		if (res == SQL_ERROR) {
+			ast_odbc_print_errors(SQL_HANDLE_STMT, stmt, "SQL Prepare");
+		}
 		ast_log(LOG_WARNING, "SQL Prepare failed! [%s]\n", ast_str_buffer(sql));
 		SQLFreeHandle(SQL_HANDLE_STMT, stmt);
 		return NULL;
diff --git a/res/res_odbc.c b/res/res_odbc.c
index b4c1585..1c82e3f 100644
--- a/res/res_odbc.c
+++ b/res/res_odbc.c
@@ -431,23 +431,20 @@
 {
 	struct ast_str *errors = ast_str_thread_get(&errors_buf, 16);
 	SQLINTEGER nativeerror = 0;
-	SQLINTEGER numfields = 0;
 	SQLSMALLINT diagbytes = 0;
 	SQLSMALLINT i;
 	unsigned char state[10];
 	unsigned char diagnostic[256];
 
 	ast_str_reset(errors);
-	SQLGetDiagField(handle_type, handle, 1, SQL_DIAG_NUMBER, &numfields,
-			SQL_IS_INTEGER, &diagbytes);
-	for (i = 0; i < numfields; i++) {
-		SQLGetDiagRec(handle_type, handle, i + 1, state, &nativeerror,
-				diagnostic, sizeof(diagnostic), &diagbytes);
+	i = 0;
+	while (SQLGetDiagRec(handle_type, handle, ++i, state, &nativeerror,
+		diagnostic, sizeof(diagnostic), &diagbytes) == SQL_SUCCESS) {
 		ast_str_append(&errors, 0, "%s%s", ast_str_strlen(errors) ? "," : "", state);
 		ast_log(LOG_WARNING, "%s returned an error: %s: %s\n", operation, state, diagnostic);
 		/* XXX Why is this here? */
 		if (i > 10) {
-			ast_log(LOG_WARNING, "Oh, that was good.  There are really %d diagnostics?\n", (int)numfields);
+			ast_log(LOG_WARNING, "There are more than 10 diagnostic records! Ignore the rest.\n");
 			break;
 		}
 	}

-- 
To view, visit https://gerrit.asterisk.org/10285
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Iba5ae5470ac49ecd911dd084effbe9efac68ccc1
Gerrit-Change-Number: 10285
Gerrit-PatchSet: 1
Gerrit-Owner: Alexei Gradinari <alex2grad at gmail.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2 (1000185)
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Sean Bright <sean.bright at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180928/4b1e46d5/attachment.html>


More information about the asterisk-code-review mailing list