From fbedad84af15a1c752f89a4e436382a1b4d9dfb5 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 10 Mar 2015 17:08:04 +0200 Subject: [PATCH] Fixes to Coverity defects. --- server/core/config.c | 4 ++-- server/core/dbusers.c | 2 +- server/core/load_utils.c | 8 +++++++- .../modules/routing/maxinfo/maxinfo_parse.c | 4 +++- .../routing/schemarouter/schemarouter.c | 17 +++++++++-------- .../routing/schemarouter/shardrouter.c | 19 +++++++++---------- .../schemarouter/test/testschemarouter2.c | 2 +- 7 files changed, 32 insertions(+), 24 deletions(-) diff --git a/server/core/config.c b/server/core/config.c index db9d38fed..7723ae596 100644 --- a/server/core/config.c +++ b/server/core/config.c @@ -2138,7 +2138,7 @@ config_get_release_string(char* release) *end = 0; to = strcpy(distribution, "lsb: "); - memmove(to, found, end - found + 1); + memmove(to, found, end - found + 1 < INT_MAX ? end - found + 1 : INT_MAX); strncpy(release, to, _RELEASE_STR_LENGTH); @@ -2175,7 +2175,7 @@ config_get_release_string(char* release) +5 and -8 below cut the file name part out of the full pathname that corresponds to the mask as above. */ - new_to = strcpy(distribution, found.gl_pathv[0] + 5); + new_to = strncpy(distribution, found.gl_pathv[0] + 5,_RELEASE_STR_LENGTH - 1); new_to += 8; *new_to++ = ':'; *new_to++ = ' '; diff --git a/server/core/dbusers.c b/server/core/dbusers.c index 9f4528eb5..d2211ac6e 100644 --- a/server/core/dbusers.c +++ b/server/core/dbusers.c @@ -798,7 +798,7 @@ getUsers(SERVICE *service, USERS *users) if (db_grants) { /* we have dbgrants, store them */ if(row[5]){ - strcpy(dbnm,row[5]); + strncpy(dbnm,row[5],MYSQL_DATABASE_MAXLEN); if(service->strip_db_esc) { strip_escape_chars(dbnm); } diff --git a/server/core/load_utils.c b/server/core/load_utils.c index 2298df581..6238c1255 100644 --- a/server/core/load_utils.c +++ b/server/core/load_utils.c @@ -690,6 +690,8 @@ module_create_feedback_report(GWBUF **buffer, MODULES *modules, FEEDBACK_CONF *c time_t now; struct tm *now_tm; int report_max_bytes=0; + if(buffer == NULL) + return 0; now = time(NULL); @@ -838,13 +840,13 @@ do_http_post(GWBUF *buffer, void *cfg) { /* Check for errors */ if(res != CURLE_OK) { ret_code = 2; - LOGIF(LE, (skygw_log_write_flush( LOGFILE_ERROR, "Error: do_http_post(), curl call for [%s] failed due: %s, %s", feedback_config->feedback_url, curl_easy_strerror(res), error_message))); + goto cleanup; } else { curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &http_code); } @@ -855,6 +857,7 @@ do_http_post(GWBUF *buffer, void *cfg) { ret_code = 0; } else { ret_code = 3; + goto cleanup; } } else { LOGIF(LE, (skygw_log_write_flush( @@ -862,18 +865,21 @@ do_http_post(GWBUF *buffer, void *cfg) { "Error: do_http_post(), Bad HTTP Code from remote server: %lu", http_code))); ret_code = 4; + goto cleanup; } } else { LOGIF(LE, (skygw_log_write_flush( LOGFILE_ERROR, "Error: do_http_post(), curl object not initialized"))); ret_code = 1; + goto cleanup; } LOGIF(LT, (skygw_log_write_flush( LOGFILE_TRACE, "do_http_post() ret_code [%d], HTTP code [%d]", ret_code, http_code))); + cleanup: if (chunk.data) free(chunk.data); diff --git a/server/modules/routing/maxinfo/maxinfo_parse.c b/server/modules/routing/maxinfo/maxinfo_parse.c index d7035dcc2..9eb445a22 100644 --- a/server/modules/routing/maxinfo/maxinfo_parse.c +++ b/server/modules/routing/maxinfo/maxinfo_parse.c @@ -126,7 +126,7 @@ MAXINFO_TREE *col, *table; /** * Parse a column list, may be a * or a valid list of string name - * seperated by a comma + * separated by a comma * * @param sql Pointer to pointer to column list updated to point to the table name * @return A tree of column names @@ -148,9 +148,11 @@ MAXINFO_TREE * rval = NULL; case LT_COMMA: rval = make_tree_node(MAXOP_COLUMNS, text, NULL, parse_column_list(ptr)); + break; case LT_FROM: rval = make_tree_node(MAXOP_COLUMNS, text, NULL, NULL); + break; default: break; } diff --git a/server/modules/routing/schemarouter/schemarouter.c b/server/modules/routing/schemarouter/schemarouter.c index d2e7de4e8..4baa795b3 100644 --- a/server/modules/routing/schemarouter/schemarouter.c +++ b/server/modules/routing/schemarouter/schemarouter.c @@ -1632,7 +1632,7 @@ gen_show_dbs_response(ROUTER_INSTANCE* router, ROUTER_CLIENT_SES* client) rval = gwbuf_append(rval, last_packet); rval = gwbuf_make_contiguous(rval); - + hashtable_iterator_free(iter); return rval; } @@ -2224,19 +2224,20 @@ static void clientReply ( goto lock_failed; } bref = get_bref_from_dcb(router_cli_ses, backend_dcb); - skygw_log_write(LOGFILE_DEBUG,"schemarouter: Received reply from %s for session %p", - bref->bref_backend->backend_server->unique_name, - router_cli_ses->rses_client_dcb->session); -#if !defined(FOR_BUG548_FIX_ONLY) - /** This makes the issue becoming visible in poll.c */ + if (bref == NULL) { /** Unlock router session */ rses_end_locked_router_action(router_cli_ses); goto lock_failed; } -#endif + skygw_log_write(LOGFILE_DEBUG,"schemarouter: Received reply from %s for session %p", + bref->bref_backend->backend_server->unique_name, + router_cli_ses->rses_client_dcb->session); + + + if(router_cli_ses->init & INIT_MAPPING) { bool mapped = true, logged = false; @@ -3905,7 +3906,7 @@ static bool handle_error_new_connection( skygw_log_write(LOGFILE_TRACE,"schemarouter: Re-mapping databases"); gen_databaselist(rses->router,rses); - + hashtable_iterator_free(iter); return_succp: return succp; } diff --git a/server/modules/routing/schemarouter/shardrouter.c b/server/modules/routing/schemarouter/shardrouter.c index 8b82e4b3c..c4a9e88ed 100644 --- a/server/modules/routing/schemarouter/shardrouter.c +++ b/server/modules/routing/schemarouter/shardrouter.c @@ -274,7 +274,8 @@ char* get_lenenc_str(void* data, int* len) if(data == NULL || len == NULL) { - *len = -1; + if(len) + *len = -1; return NULL; } @@ -1099,7 +1100,8 @@ return_rses: } #endif errorblock: - if(client_rses->subservice) + + if(client_rses && client_rses->subservice) { for(j = 0; j < i; j++) { @@ -1661,14 +1663,7 @@ routeQuery(ROUTER* instance, ret = 1; } - else - { - /** Something else went wrong, terminate connection */ - ret = 0; - } - goto retblock; - } } @@ -2794,6 +2789,9 @@ get_shard_subsvc(SUBSERVICE** subsvc,ROUTER_CLIENT_SES* session,char* target) { int i; + if(subsvc == NULL || session == NULL || target == NULL) + return false; + for(i = 0;in_subservice;i++) { if(strcmp(session->subservice[i]->service->name,target) == 0) @@ -2859,7 +2857,7 @@ router_handle_state_switch( CHK_DCB(dcb); return rc; - +#if 0 if(SERVER_IS_RUNNING(srv) && SERVER_IS_IN_CLUSTER(srv)) { goto return_rc; @@ -2882,6 +2880,7 @@ router_handle_state_switch( return_rc: return rc; +#endif } /** diff --git a/server/modules/routing/schemarouter/test/testschemarouter2.c b/server/modules/routing/schemarouter/test/testschemarouter2.c index 00c0a5bf6..a7dd1d94d 100644 --- a/server/modules/routing/schemarouter/test/testschemarouter2.c +++ b/server/modules/routing/schemarouter/test/testschemarouter2.c @@ -78,7 +78,7 @@ int main(int argc, char** argv) goto report; } - sprintf(query,"STOP SLAVE",databases[i]); + sprintf(query,"STOP SLAVE"); if(mysql_real_query(server,query,strlen(query))) { fprintf(stderr, "Failed to stop slave in %d: %s.\n",