[asterisk-commits] jrose: branch 1.8 r362354 - in /branches/1.8: main/ res/

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Tue Apr 17 15:43:29 CDT 2012


Author: jrose
Date: Tue Apr 17 15:43:22 2012
New Revision: 362354

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=362354
Log:
Make use of va_args more appropriate to form in various res_config modules plus utils.

A number of va_copy operations weren't matched with a corresponding va_end in res_config_odbc. Also, there was a potential for va_end to be invoked twice on the same va_arg in utils, which would mean invoking va_end on an undefined variable... which is bad.
va_end is removed from various functions in config_pgsql and config_curl since they aren't making their own copy.  The invokers of those functions are responsible for calling va_end on them.

(closes issue ASTERISK-19451)
Reported by: Walter Doekes
Review: https://reviewboard.asterisk.org/r/1848/

Modified:
    branches/1.8/main/utils.c
    branches/1.8/res/res_config_curl.c
    branches/1.8/res/res_config_odbc.c
    branches/1.8/res/res_config_pgsql.c

Modified: branches/1.8/main/utils.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/main/utils.c?view=diff&rev=362354&r1=362353&r2=362354
==============================================================================
--- branches/1.8/main/utils.c (original)
+++ branches/1.8/main/utils.c Tue Apr 17 15:43:22 2012
@@ -1754,8 +1754,6 @@
 	}
 
 	needed = vsnprintf(target, available, format, ap1) + 1;
-
-	va_end(ap1);
 
 	if (needed > available) {
 		/* the allocation could not be satisfied using the field's current allocation

Modified: branches/1.8/res/res_config_curl.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/res/res_config_curl.c?view=diff&rev=362354&r1=362353&r2=362354
==============================================================================
--- branches/1.8/res/res_config_curl.c (original)
+++ branches/1.8/res/res_config_curl.c Tue Apr 17 15:43:22 2012
@@ -89,7 +89,6 @@
 		ast_uri_encode(newval, buf2, sizeof(buf2), EncodeSpecialChars);
 		ast_str_append(&query, 0, "%s%s=%s", i > 0 ? "&" : "", buf1, buf2);
 	}
-	va_end(ap);
 
 	ast_str_append(&query, 0, ")}");
 	ast_str_substitute_variables(&buffer, 0, NULL, ast_str_buffer(query));
@@ -170,7 +169,6 @@
 		ast_uri_encode(newval, buf2, sizeof(buf2), EncodeSpecialChars);
 		ast_str_append(&query, 0, "%s%s=%s", i > 0 ? "&" : "", buf1, buf2);
 	}
-	va_end(ap);
 
 	ast_str_append(&query, 0, ")}");
 
@@ -261,7 +259,6 @@
 		ast_uri_encode(newval, buf2, sizeof(buf2), EncodeSpecialChars);
 		ast_str_append(&query, 0, "%s%s=%s", i > 0 ? "&" : "", buf1, buf2);
 	}
-	va_end(ap);
 
 	ast_str_append(&query, 0, ")}");
 	ast_str_substitute_variables(&buffer, 0, NULL, ast_str_buffer(query));
@@ -321,7 +318,6 @@
 		ast_str_append(&query, 0, "%s%s=%s", first ? "" : "&", buf1, buf2);
 		first = 0;
 	}
-	va_end(ap);
 
 	ast_str_append(&query, 0, ")}");
 	/* Proxies work, by setting CURLOPT options in the [globals] section of
@@ -387,7 +383,6 @@
 		ast_uri_encode(newval, buf2, sizeof(buf2), EncodeSpecialChars);
 		ast_str_append(&query, 0, "%s%s=%s", i > 0 ? "&" : "", buf1, buf2);
 	}
-	va_end(ap);
 
 	ast_str_append(&query, 0, ")}");
 	ast_str_substitute_variables(&buffer, 0, NULL, ast_str_buffer(query));
@@ -452,7 +447,6 @@
 		ast_uri_encode(newval, buf2, sizeof(buf2), EncodeSpecialChars);
 		ast_str_append(&query, 0, "%s%s=%s", i > 0 ? "&" : "", buf1, buf2);
 	}
-	va_end(ap);
 
 	ast_str_append(&query, 0, ")}");
 	ast_str_substitute_variables(&buffer, 0, NULL, ast_str_buffer(query));
@@ -514,7 +508,6 @@
 			type == RQ_FLOAT ? "float" :
 			"unknown", size);
 	}
-	va_end(ap);
 
 	ast_str_append(&query, 0, ")}");
 	ast_str_substitute_variables(&buffer, 0, NULL, ast_str_buffer(query));

Modified: branches/1.8/res/res_config_odbc.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/res/res_config_odbc.c?view=diff&rev=362354&r1=362353&r2=362354
==============================================================================
--- branches/1.8/res/res_config_odbc.c (original)
+++ branches/1.8/res/res_config_odbc.c Tue Apr 17 15:43:22 2012
@@ -78,8 +78,6 @@
 	SQLHSTMT stmt;
 	va_list ap;
 
-	va_copy(ap, cps->ap);
-
 	res = SQLAllocHandle(SQL_HANDLE_STMT, obj->con, &stmt);
 	if ((res != SQL_SUCCESS) && (res != SQL_SUCCESS_WITH_INFO)) {
 		ast_log(LOG_WARNING, "SQL Alloc Handle failed!\n");
@@ -95,6 +93,7 @@
 		return NULL;
 	}
 
+	va_copy(ap, cps->ap);
 	while ((newparam = va_arg(ap, const char *))) {
 		newval = va_arg(ap, const char *);
 		if ((1LL << count++) & cps->skip) {
@@ -172,8 +171,6 @@
 	if (ast_string_field_init(&cps, 256)) {
 		return NULL;
 	}
-	va_copy(cps.ap, ap);
-	va_copy(aq, ap);
 
 	if (!table) {
 		ast_string_field_free_memory(&cps);
@@ -188,8 +185,10 @@
 		return NULL;
 	}
 
+	va_copy(aq, ap);
 	newparam = va_arg(aq, const char *);
 	if (!newparam) {
+		va_end(aq);
 		ast_odbc_release_obj(obj);
 		ast_string_field_free_memory(&cps);
 		return NULL;
@@ -206,7 +205,9 @@
 	}
 	va_end(aq);
 
+	va_copy(cps.ap, ap);
 	stmt = ast_odbc_prepare_and_execute(obj, custom_prepare, &cps);
+	va_end(cps.ap);
 
 	if (!stmt) {
 		ast_odbc_release_obj(obj);
@@ -340,8 +341,6 @@
 	if (!table || ast_string_field_init(&cps, 256)) {
 		return NULL;
 	}
-	va_copy(cps.ap, ap);
-	va_copy(aq, ap);
 
 
 	obj = ast_odbc_request_obj2(database, connected_flag);
@@ -350,15 +349,20 @@
 		return NULL;
 	}
 
+	va_copy(aq, ap);
 	newparam = va_arg(aq, const char *);
 	if (!newparam)  {
-		ast_odbc_release_obj(obj);
-		ast_string_field_free_memory(&cps);
-		return NULL;
-	}
+		va_end(aq);
+		ast_odbc_release_obj(obj);
+		ast_string_field_free_memory(&cps);
+		return NULL;
+	}
+
 	initfield = ast_strdupa(newparam);
-	if ((op = strchr(initfield, ' '))) 
+	if ((op = strchr(initfield, ' '))) {
 		*op = '\0';
+	}
+
 	va_arg(aq, const char *);
 	op = !strchr(newparam, ' ') ? " =" : "";
 	snprintf(sql, sizeof(sql), "SELECT * FROM %s WHERE %s%s ?%s", table, newparam, op,
@@ -369,11 +373,15 @@
 			strcasestr(newparam, "LIKE") && !ast_odbc_backslash_is_escape(obj) ? " ESCAPE '\\'" : "");
 		va_arg(aq, const char *);
 	}
-	if (initfield)
+	va_end(aq);
+
+	if (initfield) {
 		snprintf(sql + strlen(sql), sizeof(sql) - strlen(sql), " ORDER BY %s", initfield);
-	va_end(aq);
-
+	}
+
+	va_copy(cps.ap, ap);
 	stmt = ast_odbc_prepare_and_execute(obj, custom_prepare, &cps);
+	va_end(cps.ap);
 
 	if (!stmt) {
 		ast_odbc_release_obj(obj);
@@ -490,9 +498,6 @@
 		return -1;
 	}
 
-	va_copy(cps.ap, ap);
-	va_copy(aq, ap);
-
 	if (ast_string_field_init(&cps, 256)) {
 		return -1;
 	}
@@ -504,8 +509,10 @@
 		return -1;
 	}
 
+	va_copy(aq, ap);
 	newparam = va_arg(aq, const char *);
 	if (!newparam)  {
+		va_end(aq);
 		ast_odbc_release_obj(obj);
 		ast_odbc_release_table(tableptr);
 		ast_string_field_free_memory(&cps);
@@ -540,7 +547,9 @@
 	snprintf(sql + strlen(sql), sizeof(sql) - strlen(sql), " WHERE %s=?", keyfield);
 	ast_odbc_release_table(tableptr);
 
+	va_copy(cps.ap, ap);
 	stmt = ast_odbc_prepare_and_execute(obj, custom_prepare, &cps);
+	va_end(cps.ap);
 
 	if (!stmt) {
 		ast_odbc_release_obj(obj);
@@ -622,14 +631,16 @@
 	}
 	va_end(ap);
 
+	ast_str_append(&sql, 0, "WHERE");
+	first = 1;
+
 	/* Restart search, because we need to add the search parameters */
 	va_copy(ap, ups->ap);
-	ast_str_append(&sql, 0, "WHERE");
-	first = 1;
 
 	while ((newparam = va_arg(ap, const char *))) {
 		newval = va_arg(ap, const char *);
 		if (!(column = ast_odbc_find_column(tableptr, newparam))) {
+			va_end(ap);
 			ast_log(LOG_ERROR, "One or more of the criteria columns '%s' on '%s@%s' for this update does not exist!\n", newparam, ups->table, ups->database);
 			ast_odbc_release_table(tableptr);
 			SQLFreeHandle(SQL_HANDLE_STMT, stmt);
@@ -677,16 +688,17 @@
 	int res;
 	SQLLEN rowcount = 0;
 
+	if (!(obj = ast_odbc_request_obj(database, 0))) {
+		return -1;
+	}
+
 	va_copy(ups.ap, ap);
-
-	if (!(obj = ast_odbc_request_obj(database, 0))) {
-		return -1;
-	}
-
 	if (!(stmt = ast_odbc_prepare_and_execute(obj, update2_prepare, &ups))) {
-		ast_odbc_release_obj(obj);
-		return -1;
-	}
+		va_end(ups.ap);
+		ast_odbc_release_obj(obj);
+		return -1;
+	}
+	va_end(ups.ap);
 
 	res = SQLRowCount(stmt, &rowcount);
 	SQLFreeHandle(SQL_HANDLE_STMT, stmt);
@@ -733,18 +745,20 @@
 	struct custom_prepare_struct cps = { .sql = sql, .extra = NULL };
 	struct ast_flags connected_flag = { RES_ODBC_CONNECTED };
 
-	va_copy(cps.ap, ap);
+	if (!table) {
+		return -1;
+	}
+
+	obj = ast_odbc_request_obj2(database, connected_flag);
+	if (!obj) {
+		return -1;
+	}
+
 	va_copy(aq, ap);
-	
-	if (!table)
-		return -1;
-
-	obj = ast_odbc_request_obj2(database, connected_flag);
-	if (!obj)
-		return -1;
 
 	newparam = va_arg(aq, const char *);
 	if (!newparam)  {
+		va_end(aq);
 		ast_odbc_release_obj(obj);
 		return -1;
 	}
@@ -759,7 +773,10 @@
 	va_end(aq);
 	snprintf(sql, sizeof(sql), "INSERT INTO %s (%s) VALUES (%s)", table, keys, vals);
 
+
+	va_copy(cps.ap, ap);
 	stmt = ast_odbc_prepare_and_execute(obj, custom_prepare, &cps);
+	va_end(cps.ap);
 
 	if (!stmt) {
 		ast_odbc_release_obj(obj);
@@ -808,17 +825,18 @@
 	struct custom_prepare_struct cps = { .sql = sql, .extra = lookup };
 	struct ast_flags connected_flag = { RES_ODBC_CONNECTED };
 
-	va_copy(cps.ap, ap);
+	if (!table) {
+		return -1;
+	}
+
+	obj = ast_odbc_request_obj2(database, connected_flag);
+	if (!obj) {
+		return -1;
+	}
+
+	snprintf(sql, sizeof(sql), "DELETE FROM %s WHERE ", table);
+
 	va_copy(aq, ap);
-	
-	if (!table)
-		return -1;
-
-	obj = ast_odbc_request_obj2(database, connected_flag);
-	if (!obj)
-		return -1;
-
-	snprintf(sql, sizeof(sql), "DELETE FROM %s WHERE ", table);
 	while((newparam = va_arg(aq, const char *))) {
 		snprintf(sql + strlen(sql), sizeof(sql) - strlen(sql), "%s=? AND ", newparam);
 		va_arg(aq, const char *);
@@ -826,7 +844,9 @@
 	va_end(aq);
 	snprintf(sql + strlen(sql), sizeof(sql) - strlen(sql), "%s=?", keyfield);
 
+	va_copy(cps.ap, ap);
 	stmt = ast_odbc_prepare_and_execute(obj, custom_prepare, &cps);
+	va_end(cps.ap);
 
 	if (!stmt) {
 		ast_odbc_release_obj(obj);
@@ -1124,7 +1144,6 @@
 			ast_log(LOG_WARNING, "Realtime table %s@%s requires column '%s', but that column does not exist!\n", table, database, elm);
 		}
 	}
-	va_end(ap);
 	AST_RWLIST_UNLOCK(&tableptr->columns);
 	return 0;
 }

Modified: branches/1.8/res/res_config_pgsql.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/res/res_config_pgsql.c?view=diff&rev=362354&r1=362353&r2=362354
==============================================================================
--- branches/1.8/res/res_config_pgsql.c (original)
+++ branches/1.8/res/res_config_pgsql.c Tue Apr 17 15:43:22 2012
@@ -348,7 +348,6 @@
 	ESCAPE_STRING(escapebuf, newval);
 	if (pgresult) {
 		ast_log(LOG_ERROR, "Postgres detected invalid input: '%s'\n", newval);
-		va_end(ap);
 		return NULL;
 	}
 
@@ -363,13 +362,11 @@
 		ESCAPE_STRING(escapebuf, newval);
 		if (pgresult) {
 			ast_log(LOG_ERROR, "Postgres detected invalid input: '%s'\n", newval);
-			va_end(ap);
 			return NULL;
 		}
 
 		ast_str_append(&sql, 0, " AND %s%s '%s'", newparam, op, ast_str_buffer(escapebuf));
 	}
-	va_end(ap);
 
 	/* We now have our complete statement; Lets connect to the server and execute it. */
 	ast_mutex_lock(&pgsql_lock);
@@ -506,7 +503,6 @@
 	ESCAPE_STRING(escapebuf, newval);
 	if (pgresult) {
 		ast_log(LOG_ERROR, "Postgres detected invalid input: '%s'\n", newval);
-		va_end(ap);
 		ast_config_destroy(cfg);
 		return NULL;
 	}
@@ -522,7 +518,6 @@
 		ESCAPE_STRING(escapebuf, newval);
 		if (pgresult) {
 			ast_log(LOG_ERROR, "Postgres detected invalid input: '%s'\n", newval);
-			va_end(ap);
 			ast_config_destroy(cfg);
 			return NULL;
 		}
@@ -534,7 +529,6 @@
 		ast_str_append(&sql, 0, " ORDER BY %s", initfield);
 	}
 
-	va_end(ap);
 
 	/* We now have our complete statement; Lets connect to the server and execute it. */
 	ast_mutex_lock(&pgsql_lock);
@@ -678,7 +672,6 @@
 	ESCAPE_STRING(escapebuf, newval);
 	if (pgresult) {
 		ast_log(LOG_ERROR, "Postgres detected invalid input: '%s'\n", newval);
-		va_end(ap);
 		release_table(table);
 		return -1;
 	}
@@ -695,20 +688,17 @@
 		ESCAPE_STRING(escapebuf, newval);
 		if (pgresult) {
 			ast_log(LOG_ERROR, "Postgres detected invalid input: '%s'\n", newval);
-			va_end(ap);
 			release_table(table);
 			return -1;
 		}
 
 		ast_str_append(&sql, 0, ", %s = '%s'", newparam, ast_str_buffer(escapebuf));
 	}
-	va_end(ap);
 	release_table(table);
 
 	ESCAPE_STRING(escapebuf, lookup);
 	if (pgresult) {
 		ast_log(LOG_ERROR, "Postgres detected invalid input: '%s'\n", lookup);
-		va_end(ap);
 		return -1;
 	}
 
@@ -955,7 +945,6 @@
 		ESCAPE_STRING(buf, newval);
 		ast_str_append(&sql2, 0, ", '%s'", ast_str_buffer(buf));
 	}
-	va_end(ap);
 	ast_str_append(&sql1, 0, "%s)", ast_str_buffer(sql2));
 
 	ast_debug(1, "PostgreSQL RealTime: Insert SQL: %s\n", ast_str_buffer(sql1));
@@ -1054,7 +1043,6 @@
 		ESCAPE_STRING(buf2, newval);
 		ast_str_append(&sql, 0, " AND %s = '%s'", ast_str_buffer(buf1), ast_str_buffer(buf2));
 	}
-	va_end(ap);
 
 	ast_debug(1, "PostgreSQL RealTime: Delete SQL: %s\n", ast_str_buffer(sql));
 




More information about the asterisk-commits mailing list