[svn-commits] wdoekes: branch 12 r407968 - /branches/12/main/config.c

[SVN commits to the Digium repositories]

Author: wdoekes
Date: Wed Feb 12 02:18:37 2014
New Revision: 407968

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=407968
Log:
realtime: Fix ast_update2_realtime() on raspberry pi.

The old code depended on undefined va_arg behaviour: calling a function
twice with the same va_list parameter and expecting it to continue where
it left off. The changed code behaves like the manpage says it should.

Also added a bunch of early returns to trap errors (e.g. OOM) instead of
crashing.

The problem was found by Julian Lyndon-Smith. The deviant behaviour on
the raspberry PI also uncovered another bug (fixed in r407875) in the
res_config_pgsql.so driver.

Reported by: jmls
Tested by: jmls
Review: https://reviewboard.asterisk.org/r/3201/

Modified:
    branches/12/main/config.c

Modified: branches/12/main/config.c
URL: http://svnview.digium.com/svn/asterisk/branches/12/main/config.c?view=diff&rev=407968&r1=407967&r2=407968
==============================================================================
--- branches/12/main/config.c (original)
+++ branches/12/main/config.c Wed Feb 12 02:18:37 2014
@@ -2569,10 +2569,53 @@
 	return result;
 }
 
-static struct ast_variable *realtime_arguments_to_fields(va_list ap)
+#define realtime_arguments_to_fields(ap) realtime_arguments_to_fields2(ap, 0)
+
+static struct ast_variable *realtime_arguments_to_fields2(va_list ap, int skip)
 {
 	struct ast_variable *first, *fields = NULL;
-	const char *newparam = va_arg(ap, const char *), *newval = va_arg(ap, const char *);
+	const char *newparam;
+	const char *newval;
+
+	/*
+	 * Previously we would do:
+	 *
+	 *     va_start(ap, last);
+	 *     x = realtime_arguments_to_fields(ap);
+	 *     y = realtime_arguments_to_fields(ap);
+	 *     va_end(ap);
+	 *
+	 * While this works on generic amd64 machines (2014), it doesn't on the
+	 * raspberry PI. The va_arg() manpage says:
+	 *
+	 *     If ap is passed to a function that uses va_arg(ap,type) then
+	 *     the value of ap is undefined after the return of that function.
+	 *
+	 * On the raspberry, ap seems to get reset after the call: the contents
+	 * of y would be equal to the contents of x.
+	 *
+	 * So, instead we allow the caller to skip past earlier argument sets
+	 * using the skip parameter:
+	 *
+	 *     va_start(ap, last);
+	 *     x = realtime_arguments_to_fields(ap);
+	 *     va_end(ap);
+	 *     va_start(ap, last);
+	 *     y = realtime_arguments_to_fields2(ap, 1);
+	 *     va_end(ap);
+	 */
+	while (skip--) {
+		/* There must be at least one argument. */
+		newparam = va_arg(ap, const char *);
+		newval = va_arg(ap, const char *);
+		while ((newparam = va_arg(ap, const char *))) {
+			newval = va_arg(ap, const char *);
+		}
+	}
+
+	/* Load up the first vars. */
+	newparam = va_arg(ap, const char *);
+	newval = va_arg(ap, const char *);
 
 	if (!(first = ast_variable_new(newparam, newval, ""))) {
 		return NULL;
@@ -2680,6 +2723,10 @@
 	fields = realtime_arguments_to_fields(ap);
 	va_end(ap);
 
+	if (!fields) {
+		return NULL;
+	}
+
 	return ast_load_realtime_fields(family, fields);
 }
 
@@ -2782,6 +2829,10 @@
 	fields = realtime_arguments_to_fields(ap);
 	va_end(ap);
 
+	if (!fields) {
+		return NULL;
+	}
+
 	return ast_load_realtime_multientry_fields(family, fields);
 }
 
@@ -2815,6 +2866,10 @@
 	fields = realtime_arguments_to_fields(ap);
 	va_end(ap);
 
+	if (!fields) {
+		return -1;
+	}
+
 	return ast_update_realtime_fields(family, keyfield, lookup, fields);
 }
 
@@ -2845,9 +2900,19 @@
 	va_list ap;
 
 	va_start(ap, family);
+	/* XXX: If we wanted to pass no lookup fields (select all), we'd be
+	 * out of luck. realtime_arguments_to_fields expects at least one key
+	 * value pair. */
 	lookup_fields = realtime_arguments_to_fields(ap);
-	update_fields = realtime_arguments_to_fields(ap);
 	va_end(ap);
+
+	va_start(ap, family);
+	update_fields = realtime_arguments_to_fields2(ap, 1);
+	va_end(ap);
+
+	if (!lookup_fields || !update_fields) {
+		return -1;
+	}
 
 	return ast_update2_realtime_fields(family, lookup_fields, update_fields);
 }
@@ -2882,6 +2947,10 @@
 	fields = realtime_arguments_to_fields(ap);
 	va_end(ap);
 
+	if (!fields) {
+		return -1;
+	}
+
 	return ast_store_realtime_fields(family, fields);
 }
 
@@ -2913,6 +2982,10 @@
 	va_start(ap, lookup);
 	fields = realtime_arguments_to_fields(ap);
 	va_end(ap);
+
+	if (!fields) {
+		return -1;
+	}
 
 	return ast_destroy_realtime_fields(family, keyfield, lookup, fields);
 }