[asterisk-commits] kmoore: branch 1.8 r365398 - in /branches/1.8: apps/ channels/ channels/sip/ ...

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Fri May 4 17:13:00 CDT 2012


Author: kmoore
Date: Fri May  4 17:12:55 2012
New Revision: 365398

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=365398
Log:
Fix many issues from the NULL_RETURNS Coverity report

Most of the changes here are trivial NULL checks.  There are a couple
optimizations to remove the need to check for NULL and outboundproxy parsing
in chan_sip.c was rewritten to avoid use of strtok.  Additionally, a bug was
found and fixed with the parsing of outboundproxy when "outboundproxy=," was
set.

(Closes issue ASTERISK-19654)

Modified:
    branches/1.8/apps/app_chanspy.c
    branches/1.8/apps/app_followme.c
    branches/1.8/apps/app_stack.c
    branches/1.8/apps/app_voicemail.c
    branches/1.8/channels/chan_iax2.c
    branches/1.8/channels/chan_sip.c
    branches/1.8/channels/sip/config_parser.c
    branches/1.8/funcs/func_aes.c
    branches/1.8/main/config.c
    branches/1.8/main/features.c
    branches/1.8/pbx/pbx_config.c

Modified: branches/1.8/apps/app_chanspy.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/apps/app_chanspy.c?view=diff&rev=365398&r1=365397&r2=365398
==============================================================================
--- branches/1.8/apps/app_chanspy.c (original)
+++ branches/1.8/apps/app_chanspy.c Fri May  4 17:12:55 2012
@@ -765,13 +765,10 @@
 	const char *context, const char *mailbox, const char *name_context)
 {
 	char nameprefix[AST_NAME_STRLEN];
-	char peer_name[AST_NAME_STRLEN + 5];
 	char exitcontext[AST_MAX_CONTEXT] = "";
 	signed char zero_volume = 0;
 	int waitms;
 	int res;
-	char *ptr;
-	int num;
 	int num_spyed_upon = 1;
 	struct ast_channel_iterator *iter = NULL;
 
@@ -861,7 +858,6 @@
 				next_channel(iter, autochan, chan), next_autochan = NULL) {
 			int igrp = !mygroup;
 			int ienf = !myenforced;
-			char *s;
 
 			if (autochan->chan == prev) {
 				ast_autochan_destroy(autochan);
@@ -946,22 +942,34 @@
 				continue;
 			}
 
-			strcpy(peer_name, "spy-");
-			strncat(peer_name, autochan->chan->name, AST_NAME_STRLEN - 4 - 1);
-			ptr = strchr(peer_name, '/');
-			*ptr++ = '\0';
-			ptr = strsep(&ptr, "-");
-
-			for (s = peer_name; s < ptr; s++)
-				*s = tolower(*s);
 
 			if (!ast_test_flag(flags, OPTION_QUIET)) {
+				char peer_name[AST_NAME_STRLEN + 5];
+				char *ptr, *s;
+
+				strcpy(peer_name, "spy-");
+				strncat(peer_name, autochan->chan->name, AST_NAME_STRLEN - 4 - 1);
+				if ((ptr = strchr(peer_name, '/'))) {
+					*ptr++ = '\0';
+					for (s = peer_name; s < ptr; s++) {
+						*s = tolower(*s);
+					}
+					if ((s = strchr(ptr, '-'))) {
+						*s = '\0';
+					}
+				}
+
 				if (ast_test_flag(flags, OPTION_NAME)) {
 					const char *local_context = S_OR(name_context, "default");
 					const char *local_mailbox = S_OR(mailbox, ptr);
-					res = ast_app_sayname(chan, local_mailbox, local_context);
+					if (local_mailbox) {
+						res = ast_app_sayname(chan, local_mailbox, local_context);
+					} else {
+						res = -1;
+					}
 				}
 				if (!ast_test_flag(flags, OPTION_NAME) || res < 0) {
+					int num;
 					if (!ast_test_flag(flags, OPTION_NOTECH)) {
 						if (ast_fileexists(peer_name, NULL, NULL) > 0) {
 							res = ast_streamfile(chan, peer_name, chan->language);
@@ -976,8 +984,9 @@
 							res = ast_say_character_str(chan, peer_name, "", chan->language);
 						}
 					}
-					if ((num = atoi(ptr)))
-						ast_say_digits(chan, atoi(ptr), "", chan->language);
+					if (ptr && (num = atoi(ptr))) {
+						ast_say_digits(chan, num, "", chan->language);
+					}
 				}
 			}
 

Modified: branches/1.8/apps/app_followme.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/apps/app_followme.c?view=diff&rev=365398&r1=365397&r2=365398
==============================================================================
--- branches/1.8/apps/app_followme.c (original)
+++ branches/1.8/apps/app_followme.c Fri May  4 17:12:55 2012
@@ -884,6 +884,10 @@
 	struct findme_user_listptr *findme_user_list;
 
 	findme_user_list = ast_calloc(1, sizeof(*findme_user_list));
+	if (!findme_user_list) {
+		ast_log(LOG_WARNING, "Failed to allocate memory for findme_user_list\n");
+		return;
+	}
 	AST_LIST_HEAD_INIT_NOLOCK(findme_user_list);
 
 	/* We're going to figure out what the longest possible string of digits to collect is */

Modified: branches/1.8/apps/app_stack.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/apps/app_stack.c?view=diff&rev=365398&r1=365397&r2=365398
==============================================================================
--- branches/1.8/apps/app_stack.c (original)
+++ branches/1.8/apps/app_stack.c Fri May  4 17:12:55 2012
@@ -764,8 +764,15 @@
 			struct ast_pbx *pbx = chan->pbx;
 			struct ast_pbx_args args;
 			struct ast_datastore *stack_store = ast_channel_datastore_find(chan, &stack_info, NULL);
-			AST_LIST_HEAD(, gosub_stack_frame) *oldlist = stack_store->data;
-			struct gosub_stack_frame *cur = AST_LIST_FIRST(oldlist);
+			AST_LIST_HEAD(, gosub_stack_frame) *oldlist;
+			struct gosub_stack_frame *cur;
+			if (!stack_store) {
+				ast_log(LOG_WARNING, "No GoSub stack remaining after AGI GoSub execution.\n");
+				ast_free(gosub_args);
+				return RESULT_FAILURE;
+			}
+			oldlist = stack_store->data;
+			cur = AST_LIST_FIRST(oldlist);
 			cur->is_agi = 1;
 
 			memset(&args, 0, sizeof(args));

Modified: branches/1.8/apps/app_voicemail.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/apps/app_voicemail.c?view=diff&rev=365398&r1=365397&r2=365398
==============================================================================
--- branches/1.8/apps/app_voicemail.c (original)
+++ branches/1.8/apps/app_voicemail.c Fri May  4 17:12:55 2012
@@ -11523,6 +11523,10 @@
 	if (ast_event_get_ie_uint(event, AST_EVENT_IE_EVENTTYPE) != AST_EVENT_MWI)
 		return;
 
+	if (!uniqueid) {
+		ast_log(LOG_ERROR, "Unable to allocate memory for uniqueid\n");
+		return;
+	}
 	u = ast_event_get_ie_uint(event, AST_EVENT_IE_UNIQUEID);
 	*uniqueid = u;
 	if (ast_taskprocessor_push(mwi_subscription_tps, handle_unsubscribe, uniqueid) < 0) {

Modified: branches/1.8/channels/chan_iax2.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/channels/chan_iax2.c?view=diff&rev=365398&r1=365397&r2=365398
==============================================================================
--- branches/1.8/channels/chan_iax2.c (original)
+++ branches/1.8/channels/chan_iax2.c Fri May  4 17:12:55 2012
@@ -13046,6 +13046,11 @@
 			ast_config_destroy(ucfg);
 			return 0;
 		}
+		if (!cfg) {
+			/* should have been able to load the config here */
+			ast_log(LOG_ERROR, "Unable to load config %s again\n", config_file);
+			return -1;
+		}
 	} else if (cfg == CONFIG_STATUS_FILEINVALID) {
 		ast_log(LOG_ERROR, "Config file %s is in an invalid format.  Aborting.\n", config_file);
 		return 0;
@@ -13923,8 +13928,9 @@
 	} else  if (!strncasecmp(colname, "codec[", 6)) {
 		char *codecnum, *ptr;
 		int codec = 0;
-		
-		codecnum = strchr(colname, '[');
+
+		/* skip over "codec" to the '[' */
+		codecnum = colname + 5;
 		*codecnum = '\0';
 		codecnum++;
 		if ((ptr = strchr(codecnum, ']'))) {

Modified: branches/1.8/channels/chan_sip.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/channels/chan_sip.c?view=diff&rev=365398&r1=365397&r2=365398
==============================================================================
--- branches/1.8/channels/chan_sip.c (original)
+++ branches/1.8/channels/chan_sip.c Fri May  4 17:12:55 2012
@@ -15265,7 +15265,9 @@
 		char *end_quote;
 		rname = tmp + 1;
 		end_quote = strchr(rname, '\"');
-		*end_quote = '\0';
+		if (end_quote) {
+			*end_quote = '\0';
+		}
 	}
 
 	if (number) {
@@ -24210,6 +24212,7 @@
 	 * document earlier. So there's no need to enclose this operation in an if statement.
 	 */
 	tuple_children = ast_xml_node_get_children(tuple_node);
+	/* coverity[null_returns: FALSE] */
 	status_node = ast_xml_find_element(tuple_children, "status", NULL, NULL);
 
 	if (!(status_children = ast_xml_node_get_children(status_node))) {
@@ -24541,7 +24544,8 @@
 	struct sip_peer *authpeer = NULL;
 	const char *eventheader = get_header(req, "Event");	/* Get Event package name */
 	int resubscribe = (p->subscribed != NONE) && !req->ignore;
-	char *temp, *event;
+	char *event_end;
+	ptrdiff_t event_len = 0;
 
 	if (p->initreq.headers) {	
 		/* We already have a dialog */
@@ -24603,13 +24607,10 @@
 		return 0;
 	}
 
-	if ( (strchr(eventheader, ';'))) {
-		event = ast_strdupa(eventheader);	/* Since eventheader is a const, we can't change it */
-		temp = strchr(event, ';'); 		
-		*temp = '\0';				/* Remove any options for now */
-							/* We might need to use them later :-) */
-	} else
-		event = (char *) eventheader;		/* XXX is this legal ? */
+	event_end = strchr(eventheader, ';');
+	if (event_end) {
+		event_len = event_end - eventheader;
+	}
 
 	/* Handle authentication if we're new and not a retransmission. We can't just
 	 * use if !req->ignore, because then we'll end up sending
@@ -24648,7 +24649,7 @@
 		return 0;
 	}
 
-	if (strcmp(event, "message-summary") && strcmp(event, "call-completion")) {
+	if (strncmp(eventheader, "message-summary", MAX(event_len, 15)) && strncmp(eventheader, "call-completion", MAX(event_len, 15))) {
 		/* Get destination right away */
 		gotdest = get_destination(p, NULL, NULL);
 	}
@@ -24674,7 +24675,7 @@
 	if (ast_strlen_zero(p->tag))
 		make_our_tag(p->tag, sizeof(p->tag));
 
-	if (!strcmp(event, "presence") || !strcmp(event, "dialog")) { /* Presence, RFC 3842 */
+	if (!strncmp(eventheader, "presence", MAX(event_len, 8)) || !strncmp(eventheader, "dialog", MAX(event_len, 6))) { /* Presence, RFC 3842 */
 		unsigned int pidf_xml;
 		const char *accept;
 		int start = 0;
@@ -24752,7 +24753,7 @@
 		} else {
 			p->subscribed = subscribed;
 		}
-	} else if (!strcmp(event, "message-summary")) {
+	} else if (!strncmp(eventheader, "message-summary", MAX(event_len, 15))) {
 		int start = 0;
 		int found_supported = 0;
 		const char *acceptheader;
@@ -24815,11 +24816,11 @@
 			p->relatedpeer = ref_peer(authpeer, "setting dialog's relatedpeer pointer");
 		}
 		/* Do not release authpeer here */
-	} else if (!strcmp(event, "call-completion")) {
+	} else if (!strncmp(eventheader, "call-completion", MAX(event_len, 15))) {
 		handle_cc_subscribe(p, req);
 	} else { /* At this point, Asterisk does not understand the specified event */
 		transmit_response(p, "489 Bad Event", req);
-		ast_debug(2, "Received SIP subscribe for unknown event package: %s\n", event);
+		ast_debug(2, "Received SIP subscribe for unknown event package: %s\n", eventheader);
 		pvt_set_needdestroy(p, "unknown event package");
 		if (authpeer) {
 			unref_peer(authpeer, "unref_peer, from handle_request_subscribe (authpeer 5)");
@@ -27389,32 +27390,42 @@
 			} else if (!strcasecmp(v->name, "fromuser")) {
 				ast_string_field_set(peer, fromuser, v->value);
 			} else if (!strcasecmp(v->name, "outboundproxy")) {
-				char *tok, *proxyname;
+				char *host, *proxyname, *sep;
 
 				if (ast_strlen_zero(v->value)) {
-					ast_log(LOG_WARNING, "no value given for outbound proxy on line %d of sip.conf.", v->lineno);
+					ast_log(LOG_WARNING, "no value given for outbound proxy on line %d of sip.conf\n", v->lineno);
 					continue;
 				}
 
-				peer->outboundproxy =
-				    ao2_alloc(sizeof(*peer->outboundproxy), NULL);
-
-				tok = ast_skip_blanks(strtok(ast_strdupa(v->value), ","));
-
-				sip_parse_host(tok, v->lineno, &proxyname,
-					       &peer->outboundproxy->port,
-					       &peer->outboundproxy->transport);
-
-				tok = ast_skip_blanks(strtok(ast_strdupa(v->value), ","));
-
-				if ((tok = strtok(NULL, ","))) {
-					peer->outboundproxy->force = !strncasecmp(ast_skip_blanks(tok), "force", 5);
+				if (!peer->outboundproxy) {
+					peer->outboundproxy = ao2_alloc(sizeof(*peer->outboundproxy), NULL);
+					if (!peer->outboundproxy) {
+						ast_log(LOG_WARNING, "Unable to allocate config storage for outboundproxy\n");
+						continue;
+					}
+				}
+
+				host = ast_strdupa(v->value);
+				if (!host) {
+					ast_log(LOG_WARNING, "Unable to allocate stack space for parsing outboundproxy\n");
+					continue;
+				}
+
+				host = ast_skip_blanks(host);
+				sep = strchr(host, ',');
+				if (sep) {
+					*sep++ = '\0';
+					peer->outboundproxy->force = !strncasecmp(ast_skip_blanks(sep), "force", 5);
 				} else {
 					peer->outboundproxy->force = FALSE;
 				}
 
+				sip_parse_host(host, v->lineno, &proxyname,
+					       &peer->outboundproxy->port,
+					       &peer->outboundproxy->transport);
+
 				if (ast_strlen_zero(proxyname)) {
-					ast_log(LOG_WARNING, "you must specify a name for the outboundproxy on line %d of sip.conf.", v->lineno);
+					ast_log(LOG_WARNING, "you must specify a name for the outboundproxy on line %d of sip.conf\n", v->lineno);
 					sip_cfg.outboundproxy.name[0] = '\0';
 					continue;
 				}
@@ -27968,6 +27979,11 @@
 			ast_config_destroy(ucfg);
 			return 1;
 		}
+		if (!cfg) {
+			/* should have been able to reload here */
+			ast_log(LOG_NOTICE, "Unable to load config %s\n", config);
+			return -1;
+		}
 	} else if (cfg == CONFIG_STATUS_FILEINVALID) {
 		ast_log(LOG_ERROR, "Contents of %s are invalid and cannot be parsed\n", config);
 		return 1;
@@ -28360,27 +28376,34 @@
 				default_fromdomainport = STANDARD_SIP_PORT;
 			}
 		} else if (!strcasecmp(v->name, "outboundproxy")) {
-			char *tok, *proxyname;
+			char *host, *proxyname, *sep;
 
 			if (ast_strlen_zero(v->value)) {
-				ast_log(LOG_WARNING, "no value given for outbound proxy on line %d of sip.conf.", v->lineno);
+				ast_log(LOG_WARNING, "no value given for outbound proxy on line %d of sip.conf\n", v->lineno);
 				continue;
 			}
 
-			tok = ast_skip_blanks(strtok(ast_strdupa(v->value), ","));
-
-			sip_parse_host(tok, v->lineno, &proxyname,
+			host = ast_strdupa(v->value);
+			if (!host) {
+				ast_log(LOG_WARNING, "Unable to allocate stack space for parsing outboundproxy\n");
+				continue;
+			}
+
+			host = ast_skip_blanks(host);
+			sep = strchr(host, ',');
+			if (sep) {
+				*sep++ = '\0';
+				sip_cfg.outboundproxy.force = !strncasecmp(ast_skip_blanks(sep), "force", 5);
+			} else {
+				sip_cfg.outboundproxy.force = FALSE;
+			}
+
+			sip_parse_host(host, v->lineno, &proxyname,
 				       &sip_cfg.outboundproxy.port,
 				       &sip_cfg.outboundproxy.transport);
 
-			if ((tok = strtok(NULL, ","))) {
-				sip_cfg.outboundproxy.force = !strncasecmp(ast_skip_blanks(tok), "force", 5);
-			} else {
-				sip_cfg.outboundproxy.force = FALSE;
-			}
-
 			if (ast_strlen_zero(proxyname)) {
-				ast_log(LOG_WARNING, "you must specify a name for the outboundproxy on line %d of sip.conf.", v->lineno);
+				ast_log(LOG_WARNING, "you must specify a name for the outboundproxy on line %d of sip.conf\n", v->lineno);
 				sip_cfg.outboundproxy.name[0] = '\0';
 				continue;
 			}

Modified: branches/1.8/channels/sip/config_parser.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/channels/sip/config_parser.c?view=diff&rev=365398&r1=365397&r2=365398
==============================================================================
--- branches/1.8/channels/sip/config_parser.c (original)
+++ branches/1.8/channels/sip/config_parser.c Fri May  4 17:12:55 2012
@@ -638,6 +638,7 @@
 	char *port;
 
 	if (ast_strlen_zero(line)) {
+		*hostname = NULL;
 		return -1;
 	}
 	if ((*hostname = strstr(line, "://"))) {

Modified: branches/1.8/funcs/func_aes.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/funcs/func_aes.c?view=diff&rev=365398&r1=365397&r2=365398
==============================================================================
--- branches/1.8/funcs/func_aes.c (original)
+++ branches/1.8/funcs/func_aes.c Fri May  4 17:12:55 2012
@@ -114,6 +114,10 @@
 	ast_aes_set_encrypt_key((unsigned char *) args.key, &ecx);   /* encryption:  plaintext -> encryptedtext -> base64 */
 	ast_aes_set_decrypt_key((unsigned char *) args.key, &dcx);   /* decryption:  base64 -> encryptedtext -> plaintext */
 	tmp = ast_calloc(1, len);                     /* requires a tmp buffer for the base64 decode */
+	if (!tmp) {
+		ast_log(LOG_ERROR, "Unable to allocate memory for data\n");
+		return -1;
+	}
 	tmpP = tmp;
 	encrypt = strcmp("AES_DECRYPT", cmd);           /* -1 if encrypting, 0 if decrypting */
 

Modified: branches/1.8/main/config.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/main/config.c?view=diff&rev=365398&r1=365397&r2=365398
==============================================================================
--- branches/1.8/main/config.c (original)
+++ branches/1.8/main/config.c Fri May  4 17:12:55 2012
@@ -2118,6 +2118,10 @@
 	clear_config_maps();
 
 	configtmp = ast_config_new();
+	if (!configtmp) {
+		ast_log(LOG_ERROR, "Unable to allocate memory for new config\n");
+		return -1;
+	}
 	configtmp->max_include_level = 1;
 	config = ast_config_internal_load(extconfig_conf, configtmp, flags, "", "extconfig");
 	if (config == CONFIG_STATUS_FILEINVALID) {

Modified: branches/1.8/main/features.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/main/features.c?view=diff&rev=365398&r1=365397&r2=365398
==============================================================================
--- branches/1.8/main/features.c (original)
+++ branches/1.8/main/features.c Fri May  4 17:12:55 2012
@@ -5645,7 +5645,7 @@
 static void process_applicationmap_line(struct ast_variable *var)
 {
 	char *tmp_val = ast_strdupa(var->value);
-	char *activateon;
+	char *activateon, *new_syn;
 	struct ast_call_feature *feature;
 	AST_DECLARE_APP_ARGS(args,
 		AST_APP_ARG(exten);
@@ -5656,10 +5656,10 @@
 	);
 
 	AST_STANDARD_APP_ARGS(args, tmp_val);
-	if (strchr(args.app, '(')) {
+	if ((new_syn = strchr(args.app, '('))) {
 		/* New syntax */
 		args.moh_class = args.app_args;
-		args.app_args = strchr(args.app, '(');
+		args.app_args = new_syn;
 		*args.app_args++ = '\0';
 		if (args.app_args[strlen(args.app_args) - 1] == ')') {
 			args.app_args[strlen(args.app_args) - 1] = '\0';

Modified: branches/1.8/pbx/pbx_config.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/pbx/pbx_config.c?view=diff&rev=365398&r1=365397&r2=365398
==============================================================================
--- branches/1.8/pbx/pbx_config.c (original)
+++ branches/1.8/pbx/pbx_config.c Fri May  4 17:12:55 2012
@@ -733,6 +733,11 @@
 	snprintf(filename, sizeof(filename), "%s%s%s", base, slash, config);
 
 	cfg = ast_config_load("extensions.conf", config_flags);
+	if (!cfg) {
+		ast_cli(a->fd, "Failed to load extensions.conf\n");
+		ast_mutex_unlock(&save_dialplan_lock);
+		return CLI_FAILURE;
+	}
 
 	/* try to lock contexts list */
 	if (ast_rdlock_contexts()) {




More information about the asterisk-commits mailing list