From dd81b047dbf15a501b81354db505daf50703a1a0 Mon Sep 17 00:00:00 2001 From: Kinsey Moore Date: Thu, 10 May 2012 20:56:09 +0000 Subject: Resolve FORWARD_NULL static analysis warnings This resolves core findings from ASTERISK-19650 numbers 0-2, 6, 7, 9-11, 14-20, 22-24, 28, 30-32, 34-36, 42-56, 82-84, 87, 89-90, 93-102, 104, 105, 109-111, and 115. Finding numbers 26, 33, and 29 were already resolved. Those skipped were either extended/deprecated or in areas of code that shouldn't be disturbed. (Closes issue ASTERISK-19650) ........ Merged revisions 366167 from http://svn.asterisk.org/svn/asterisk/branches/1.8 ........ Merged revisions 366168 from http://svn.asterisk.org/svn/asterisk/branches/10 git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@366169 65c4cc65-6c06-0410-ace0-fbb531ad65f3 --- main/app.c | 13 +-- main/cdr.c | 9 +- main/channel.c | 13 +-- main/config.c | 8 +- main/data.c | 22 ++--- main/devicestate.c | 2 - main/event.c | 3 +- main/features.c | 241 +++++++++++++++++++++++++++-------------------------- main/manager.c | 20 ++++- main/pbx.c | 27 +++--- main/tcptls.c | 6 +- main/xmldoc.c | 30 +++---- 12 files changed, 216 insertions(+), 178 deletions(-) (limited to 'main') diff --git a/main/app.c b/main/app.c index c37279658..3d2fa52c3 100644 --- a/main/app.c +++ b/main/app.c @@ -721,6 +721,9 @@ int ast_control_streamfile(struct ast_channel *chan, const char *file, long pause_restart_point = 0; long offset = 0; + if (!file) { + return -1; + } if (offsetms) { offset = *offsetms * 8; /* XXX Assumes 8kHz */ } @@ -752,12 +755,10 @@ int ast_control_streamfile(struct ast_channel *chan, const char *file, res = ast_answer(chan); } - if (file) { - if ((end = strchr(file, ':'))) { - if (!strcasecmp(end, ":end")) { - *end = '\0'; - end++; - } + if ((end = strchr(file, ':'))) { + if (!strcasecmp(end, ":end")) { + *end = '\0'; + end++; } } diff --git a/main/cdr.c b/main/cdr.c index f0ba2ea2e..18d53b2a3 100644 --- a/main/cdr.c +++ b/main/cdr.c @@ -581,7 +581,9 @@ void ast_cdr_merge(struct ast_cdr *to, struct ast_cdr *from) lfrom = lfrom->next; } /* rip off the last entry and put a copy of the to at the end */ - llfrom->next = to; + if (llfrom) { + llfrom->next = to; + } from = lfrom; } else { /* save copy of the current *to cdr */ @@ -597,10 +599,11 @@ void ast_cdr_merge(struct ast_cdr *to, struct ast_cdr *from) } from->next = NULL; /* rip off the last entry and put a copy of the to at the end */ - if (llfrom == from) + if (llfrom == from) { to = to->next = ast_cdr_dup(&tcdr); - else + } else if (llfrom) { to = llfrom->next = ast_cdr_dup(&tcdr); + } from = lfrom; } } diff --git a/main/channel.c b/main/channel.c index 373d6045d..3d8e22302 100644 --- a/main/channel.c +++ b/main/channel.c @@ -2967,11 +2967,6 @@ struct ast_channel *ast_waitfor_nandfds(struct ast_channel **c, int n, int *fds, int fdno; } *fdmap = NULL; - if ((sz = n * AST_MAX_FDS + nfds)) { - pfds = alloca(sizeof(*pfds) * sz); - fdmap = alloca(sizeof(*fdmap) * sz); - } - if (outfd) { *outfd = -99999; } @@ -2979,6 +2974,14 @@ struct ast_channel *ast_waitfor_nandfds(struct ast_channel **c, int n, int *fds, *exception = 0; } + if ((sz = n * AST_MAX_FDS + nfds)) { + pfds = alloca(sizeof(*pfds) * sz); + fdmap = alloca(sizeof(*fdmap) * sz); + } else { + /* nothing to allocate and no FDs to check */ + return NULL; + } + /* Perform any pending masquerades */ for (x = 0; x < n; x++) { if (ast_channel_masq(c[x]) && ast_do_masquerade(c[x])) { diff --git a/main/config.c b/main/config.c index a143927af..249cf0403 100644 --- a/main/config.c +++ b/main/config.c @@ -562,7 +562,11 @@ struct ast_variable *ast_variable_browse(const struct ast_config *config, const { struct ast_category *cat = NULL; - if (category && config->last_browse && (config->last_browse->name == category)) { + if (!category) { + return NULL; + } + + if (config->last_browse && (config->last_browse->name == category)) { cat = config->last_browse; } else { cat = ast_category_get(config, category); @@ -1898,7 +1902,7 @@ int ast_config_text_file_save(const char *configfile, const struct ast_config *c /* Dump section with any appropriate comment */ for (cmt = cat->precomments; cmt; cmt=cmt->next) { char *cmtp = cmt->cmt; - while (*cmtp == ';' && *(cmtp+1) == '!') { + while (cmtp && *cmtp == ';' && *(cmtp+1) == '!') { char *cmtp2 = strchr(cmtp+1, '\n'); if (cmtp2) cmtp = cmtp2+1; diff --git a/main/data.c b/main/data.c index e2194b7f0..146b15453 100644 --- a/main/data.c +++ b/main/data.c @@ -2496,18 +2496,20 @@ struct ast_data_iterator *ast_data_iterator_init(struct ast_data *tree, struct ast_data *internal = tree; char *path, *ptr = NULL; + if (!elements) { + return NULL; + } + /* tree is the node we want to use to iterate? or we are going * to iterate thow an internal node? */ - if (elements) { - path = ast_strdupa(elements); - - ptr = strrchr(path, '/'); - if (ptr) { - *ptr = '\0'; - internal = data_result_get_node(tree, path); - if (!internal) { - return NULL; - } + path = ast_strdupa(elements); + + ptr = strrchr(path, '/'); + if (ptr) { + *ptr = '\0'; + internal = data_result_get_node(tree, path); + if (!internal) { + return NULL; } } diff --git a/main/devicestate.c b/main/devicestate.c index ac81523c6..840441399 100644 --- a/main/devicestate.c +++ b/main/devicestate.c @@ -330,9 +330,7 @@ static enum ast_device_state _ast_device_state(const char *device, int check_cac /* We have a provider */ number = tech; tech = NULL; - } - if (provider) { ast_debug(3, "Checking if I can find provider for \"%s\" - number: %s\n", provider, number); return getproviderstate(provider, number); } diff --git a/main/event.c b/main/event.c index f5a8e709a..9ecf71028 100644 --- a/main/event.c +++ b/main/event.c @@ -1286,8 +1286,9 @@ struct ast_event *ast_event_new(enum ast_event_type type, ...) break; } + /* realloc inside one of the append functions failed */ if (!event) { - break; + return NULL; } } diff --git a/main/features.c b/main/features.c index 4ff629fb4..cbf7c1f42 100644 --- a/main/features.c +++ b/main/features.c @@ -2166,6 +2166,9 @@ static int builtin_automonitor(struct ast_channel *chan, struct ast_channel *pee struct ast_channel *caller_chan, *callee_chan; const char *automon_message_start = NULL; const char *automon_message_stop = NULL; + const char *touch_format = NULL; + const char *touch_monitor = NULL; + const char *touch_monitor_prefix = NULL; if (!monitor_ok) { ast_log(LOG_ERROR,"Cannot record the call. The monitor application is disabled.\n"); @@ -2179,10 +2182,13 @@ static int builtin_automonitor(struct ast_channel *chan, struct ast_channel *pee } set_peers(&caller_chan, &callee_chan, peer, chan, sense); - if (caller_chan) { /* Find extra messages */ - automon_message_start = pbx_builtin_getvar_helper(caller_chan, "TOUCH_MONITOR_MESSAGE_START"); - automon_message_stop = pbx_builtin_getvar_helper(caller_chan, "TOUCH_MONITOR_MESSAGE_STOP"); + if (!caller_chan || !callee_chan) { + ast_log(LOG_NOTICE,"Cannot record the call. One or both channels have gone away.\n"); + return -1; } + /* Find extra messages */ + automon_message_start = pbx_builtin_getvar_helper(caller_chan, "TOUCH_MONITOR_MESSAGE_START"); + automon_message_stop = pbx_builtin_getvar_helper(caller_chan, "TOUCH_MONITOR_MESSAGE_STOP"); if (!ast_strlen_zero(courtesytone)) { /* Play courtesy tone if configured */ if(play_message_in_bridged_call(caller_chan, callee_chan, courtesytone) == -1) { @@ -2199,58 +2205,53 @@ static int builtin_automonitor(struct ast_channel *chan, struct ast_channel *pee return AST_FEATURE_RETURN_SUCCESS; } - if (caller_chan && callee_chan) { - const char *touch_format = pbx_builtin_getvar_helper(caller_chan, "TOUCH_MONITOR_FORMAT"); - const char *touch_monitor = pbx_builtin_getvar_helper(caller_chan, "TOUCH_MONITOR"); - const char *touch_monitor_prefix = pbx_builtin_getvar_helper(caller_chan, "TOUCH_MONITOR_PREFIX"); + touch_format = pbx_builtin_getvar_helper(caller_chan, "TOUCH_MONITOR_FORMAT"); + touch_monitor = pbx_builtin_getvar_helper(caller_chan, "TOUCH_MONITOR"); + touch_monitor_prefix = pbx_builtin_getvar_helper(caller_chan, "TOUCH_MONITOR_PREFIX"); - if (!touch_format) - touch_format = pbx_builtin_getvar_helper(callee_chan, "TOUCH_MONITOR_FORMAT"); - - if (!touch_monitor) - touch_monitor = pbx_builtin_getvar_helper(callee_chan, "TOUCH_MONITOR"); + if (!touch_format) + touch_format = pbx_builtin_getvar_helper(callee_chan, "TOUCH_MONITOR_FORMAT"); - if (!touch_monitor_prefix) - touch_monitor_prefix = pbx_builtin_getvar_helper(callee_chan, "TOUCH_MONITOR_PREFIX"); + if (!touch_monitor) + touch_monitor = pbx_builtin_getvar_helper(callee_chan, "TOUCH_MONITOR"); - if (touch_monitor) { - len = strlen(touch_monitor) + 50; - args = alloca(len); - touch_filename = alloca(len); - snprintf(touch_filename, len, "%s-%ld-%s", S_OR(touch_monitor_prefix, "auto"), (long)time(NULL), touch_monitor); - snprintf(args, len, "%s,%s,m", S_OR(touch_format, "wav"), touch_filename); - } else { - caller_chan_id = ast_strdupa(S_COR(ast_channel_caller(caller_chan)->id.number.valid, - ast_channel_caller(caller_chan)->id.number.str, ast_channel_name(caller_chan))); - callee_chan_id = ast_strdupa(S_COR(ast_channel_caller(callee_chan)->id.number.valid, - ast_channel_caller(callee_chan)->id.number.str, ast_channel_name(callee_chan))); - len = strlen(caller_chan_id) + strlen(callee_chan_id) + 50; - args = alloca(len); - touch_filename = alloca(len); - snprintf(touch_filename, len, "%s-%ld-%s-%s", S_OR(touch_monitor_prefix, "auto"), (long)time(NULL), caller_chan_id, callee_chan_id); - snprintf(args, len, "%s,%s,m", S_OR(touch_format, "wav"), touch_filename); - } + if (!touch_monitor_prefix) + touch_monitor_prefix = pbx_builtin_getvar_helper(callee_chan, "TOUCH_MONITOR_PREFIX"); - for(x = 0; x < strlen(args); x++) { - if (args[x] == '/') - args[x] = '-'; - } + if (touch_monitor) { + len = strlen(touch_monitor) + 50; + args = alloca(len); + touch_filename = alloca(len); + snprintf(touch_filename, len, "%s-%ld-%s", S_OR(touch_monitor_prefix, "auto"), (long)time(NULL), touch_monitor); + snprintf(args, len, "%s,%s,m", S_OR(touch_format, "wav"), touch_filename); + } else { + caller_chan_id = ast_strdupa(S_COR(ast_channel_caller(caller_chan)->id.number.valid, + ast_channel_caller(caller_chan)->id.number.str, ast_channel_name(caller_chan))); + callee_chan_id = ast_strdupa(S_COR(ast_channel_caller(callee_chan)->id.number.valid, + ast_channel_caller(callee_chan)->id.number.str, ast_channel_name(callee_chan))); + len = strlen(caller_chan_id) + strlen(callee_chan_id) + 50; + args = alloca(len); + touch_filename = alloca(len); + snprintf(touch_filename, len, "%s-%ld-%s-%s", S_OR(touch_monitor_prefix, "auto"), (long)time(NULL), caller_chan_id, callee_chan_id); + snprintf(args, len, "%s,%s,m", S_OR(touch_format, "wav"), touch_filename); + } - ast_verb(4, "User hit '%s' to record call. filename: %s\n", code, args); + for(x = 0; x < strlen(args); x++) { + if (args[x] == '/') + args[x] = '-'; + } - pbx_exec(callee_chan, monitor_app, args); - pbx_builtin_setvar_helper(callee_chan, "TOUCH_MONITOR_OUTPUT", touch_filename); - pbx_builtin_setvar_helper(caller_chan, "TOUCH_MONITOR_OUTPUT", touch_filename); + ast_verb(4, "User hit '%s' to record call. filename: %s\n", code, args); - if (!ast_strlen_zero(automon_message_start)) { /* Play start message for both channels */ - play_message_in_bridged_call(caller_chan, callee_chan, automon_message_start); - } + pbx_exec(callee_chan, monitor_app, args); + pbx_builtin_setvar_helper(callee_chan, "TOUCH_MONITOR_OUTPUT", touch_filename); + pbx_builtin_setvar_helper(caller_chan, "TOUCH_MONITOR_OUTPUT", touch_filename); - return AST_FEATURE_RETURN_SUCCESS; + if (!ast_strlen_zero(automon_message_start)) { /* Play start message for both channels */ + play_message_in_bridged_call(caller_chan, callee_chan, automon_message_start); } - ast_log(LOG_NOTICE,"Cannot record the call. One or both channels have gone away.\n"); - return -1; + return AST_FEATURE_RETURN_SUCCESS; } static int builtin_automixmonitor(struct ast_channel *chan, struct ast_channel *peer, struct ast_bridge_config *config, const char *code, int sense, void *data) @@ -3415,12 +3416,10 @@ static int feature_exec_app(struct ast_channel *chan, struct ast_channel *peer, ast_autoservice_start(idle); ast_autoservice_ignore(idle, AST_FRAME_DTMF_END); - if(work && idle) { - pbx_builtin_setvar_helper(work, "DYNAMIC_PEERNAME", ast_channel_name(idle)); - pbx_builtin_setvar_helper(idle, "DYNAMIC_PEERNAME", ast_channel_name(work)); - pbx_builtin_setvar_helper(work, "DYNAMIC_FEATURENAME", feature->sname); - pbx_builtin_setvar_helper(idle, "DYNAMIC_FEATURENAME", feature->sname); - } + pbx_builtin_setvar_helper(work, "DYNAMIC_PEERNAME", ast_channel_name(idle)); + pbx_builtin_setvar_helper(idle, "DYNAMIC_PEERNAME", ast_channel_name(work)); + pbx_builtin_setvar_helper(work, "DYNAMIC_FEATURENAME", feature->sname); + pbx_builtin_setvar_helper(idle, "DYNAMIC_FEATURENAME", feature->sname); if (!ast_strlen_zero(feature->moh_class)) ast_moh_start(idle, feature->moh_class, NULL); @@ -3543,7 +3542,9 @@ static int feature_interpret_helper(struct ast_channel *chan, struct ast_channel if (operation) { res = fge->feature->operation(chan, peer, config, code, sense, fge->feature); } - memcpy(feature, fge->feature, sizeof(*feature)); + if (feature) { + memcpy(feature, fge->feature, sizeof(*feature)); + } if (res != AST_FEATURE_RETURN_KEEPTRYING) { AST_RWLIST_UNLOCK(&feature_groups); break; @@ -5019,76 +5020,80 @@ static int manage_parked_call(struct parkeduser *pu, const struct pollfd *pfds, /* And take them out of the parking lot */ parking_complete = 1; } else { /* still within parking time, process descriptors */ - for (x = 0; x < AST_MAX_FDS; x++) { - struct ast_frame *f; - int y; - - if (!ast_channel_fd_isset(chan, x)) { - continue; /* nothing on this descriptor */ - } - - for (y = 0; y < nfds; y++) { - if (pfds[y].fd == ast_channel_fd(chan, x)) { - /* Found poll record! */ - break; + x = 0; + if (pfds) { + for (; x < AST_MAX_FDS; x++) { + struct ast_frame *f; + int y; + + if (!ast_channel_fd_isset(chan, x)) { + continue; /* nothing on this descriptor */ } - } - if (y == nfds) { - /* Not found */ - continue; - } - - if (!(pfds[y].revents & (POLLIN | POLLERR | POLLPRI))) { - /* Next x */ - continue; - } - - if (pfds[y].revents & POLLPRI) { - ast_set_flag(ast_channel_flags(chan), AST_FLAG_EXCEPTION); - } else { - ast_clear_flag(ast_channel_flags(chan), AST_FLAG_EXCEPTION); - } - ast_channel_fdno_set(chan, x); - - /* See if they need servicing */ - f = ast_read(pu->chan); - /* Hangup? */ - if (!f || (f->frametype == AST_FRAME_CONTROL - && f->subclass.integer == AST_CONTROL_HANGUP)) { - if (f) { - ast_frfree(f); + + for (y = 0; y < nfds; y++) { + if (pfds[y].fd == ast_channel_fd(chan, x)) { + /* Found poll record! */ + break; + } } - post_manager_event("ParkedCallGiveUp", pu); - ast_cel_report_event(pu->chan, AST_CEL_PARK_END, NULL, "ParkedCallGiveUp", - NULL); - - /* There's a problem, hang them up */ - ast_verb(2, "%s got tired of being parked\n", ast_channel_name(chan)); - ast_hangup(chan); - - /* And take them out of the parking lot */ - parking_complete = 1; - break; - } else { - /* XXX Maybe we could do something with packets, like dial "0" for operator or something XXX */ - ast_frfree(f); - if (pu->hold_method == AST_CONTROL_HOLD - && pu->moh_trys < 3 - && !ast_channel_generatordata(chan)) { - ast_debug(1, - "MOH on parked call stopped by outside source. Restarting on channel %s.\n", - ast_channel_name(chan)); - ast_indicate_data(chan, AST_CONTROL_HOLD, - S_OR(pu->parkinglot->cfg.mohclass, NULL), - (!ast_strlen_zero(pu->parkinglot->cfg.mohclass) - ? strlen(pu->parkinglot->cfg.mohclass) + 1 : 0)); - pu->moh_trys++; + if (y == nfds) { + /* Not found */ + continue; } - goto std; /* XXX Ick: jumping into an else statement??? XXX */ - } - } /* End for */ + + if (!(pfds[y].revents & (POLLIN | POLLERR | POLLPRI))) { + /* Next x */ + continue; + } + + if (pfds[y].revents & POLLPRI) { + ast_set_flag(ast_channel_flags(chan), AST_FLAG_EXCEPTION); + } else { + ast_clear_flag(ast_channel_flags(chan), AST_FLAG_EXCEPTION); + } + ast_channel_fdno_set(chan, x); + + /* See if they need servicing */ + f = ast_read(pu->chan); + /* Hangup? */ + if (!f || (f->frametype == AST_FRAME_CONTROL + && f->subclass.integer == AST_CONTROL_HANGUP)) { + if (f) { + ast_frfree(f); + } + post_manager_event("ParkedCallGiveUp", pu); + ast_cel_report_event(pu->chan, AST_CEL_PARK_END, NULL, "ParkedCallGiveUp", + NULL); + + /* There's a problem, hang them up */ + ast_verb(2, "%s got tired of being parked\n", ast_channel_name(chan)); + ast_hangup(chan); + + /* And take them out of the parking lot */ + parking_complete = 1; + break; + } else { + /* XXX Maybe we could do something with packets, like dial "0" for operator or something XXX */ + ast_frfree(f); + if (pu->hold_method == AST_CONTROL_HOLD + && pu->moh_trys < 3 + && !ast_channel_generatordata(chan)) { + ast_debug(1, + "MOH on parked call stopped by outside source. Restarting on channel %s.\n", + ast_channel_name(chan)); + ast_indicate_data(chan, AST_CONTROL_HOLD, + S_OR(pu->parkinglot->cfg.mohclass, NULL), + (!ast_strlen_zero(pu->parkinglot->cfg.mohclass) + ? strlen(pu->parkinglot->cfg.mohclass) + 1 : 0)); + pu->moh_trys++; + } + goto std; /* XXX Ick: jumping into an else statement??? XXX */ + } + } /* End for */ + } if (x >= AST_MAX_FDS) { -std: for (x = 0; x < AST_MAX_FDS; x++) { /* mark fds for next round */ +std: + for (x = 0; x < AST_MAX_FDS; x++) { /* mark fds for next round */ if (ast_channel_fd_isset(chan, x)) { void *tmp = ast_realloc(*new_pfds, (*new_nfds + 1) * sizeof(struct pollfd)); diff --git a/main/manager.c b/main/manager.c index 2a4bfa0c1..7a1c9a83b 100644 --- a/main/manager.c +++ b/main/manager.c @@ -4690,10 +4690,22 @@ static int action_reload(struct mansession *s, const struct message *m) const char *module = astman_get_header(m, "Module"); int res = ast_module_reload(S_OR(module, NULL)); - if (res == 2) { + switch (res) { + case -1: + astman_send_error(s, m, "A reload is in progress"); + break; + case 0: + astman_send_error(s, m, "No such module"); + break; + case 1: + astman_send_error(s, m, "Module does not support reload"); + break; + case 2: astman_send_ack(s, m, "Module Reloaded"); - } else { - astman_send_error(s, m, s == 0 ? "No such module" : "Module does not support reload"); + break; + default: + astman_send_error(s, m, "An unknown error occurred"); + break; } return 0; } @@ -7054,7 +7066,7 @@ static int __init_manager(int reload) if (user_writetimeout) { int value = atoi(user_writetimeout); if (value < 100) { - ast_log(LOG_WARNING, "Invalid writetimeout value '%s' at users.conf line %d\n", var->value, var->lineno); + ast_log(LOG_WARNING, "Invalid writetimeout value '%d' in users.conf\n", value); } else { user->writetimeout = value; } diff --git a/main/pbx.c b/main/pbx.c index f04051ceb..7ea360024 100644 --- a/main/pbx.c +++ b/main/pbx.c @@ -1733,8 +1733,9 @@ static void cli_match_char_tree(struct match_char *node, char *prefix, int fd) extenstr[0] = '\0'; - if (node && node->exten) + if (node->exten) { snprintf(extenstr, sizeof(extenstr), "(%p)", node->exten); + } if (strlen(node->x) > 1) { ast_cli(fd, "%s[%s]:%c:%c:%d:%s%s%s\n", prefix, node->x, node->is_pattern ? 'Y' : 'N', @@ -9705,6 +9706,11 @@ static int pbx_builtin_gotoiftime(struct ast_channel *chan, const char *data) struct timeval tv = ast_tvnow(); long timesecs; + if (!chan) { + ast_log(LOG_WARNING, "GotoIfTime requires a channel on which to operate\n"); + return -1; + } + if (ast_strlen_zero(data)) { ast_log(LOG_WARNING, "GotoIfTime requires an argument:\n