From 071e15048dd7d9e0cfd5c8dac51b80acbd5be21e Mon Sep 17 00:00:00 2001 From: MassimilianoPinto Date: Mon, 10 Nov 2014 11:08:12 +0100 Subject: [PATCH 1/4] Removed leak for service with more listeners Removed leak for service with more listeners: users are allocated once in service --- server/core/service.c | 68 +++++++++++++++++++++++-------------------- 1 file changed, 36 insertions(+), 32 deletions(-) diff --git a/server/core/service.c b/server/core/service.c index ce5261990..ae3c8edcd 100644 --- a/server/core/service.c +++ b/server/core/service.c @@ -213,42 +213,46 @@ GWPROTOCOL *funcs; if (strcmp(port->protocol, "MySQLClient") == 0) { int loaded; - /* - * Allocate specific data for MySQL users - * including hosts and db names - */ - service->users = mysql_users_alloc(); - - if ((loaded = load_mysql_users(service)) < 0) - { - LOGIF(LE, (skygw_log_write_flush( - LOGFILE_ERROR, - "Error : Unable to load users from %s:%d for " - "service %s.", - port->address, - port->port, - service->name))); - hashtable_free(service->users->data); - free(service->users); - dcb_free(port->listener); - port->listener = NULL; - goto retblock; - } - /* At service start last update is set to USERS_REFRESH_TIME seconds earlier. - * This way MaxScale could try reloading users' just after startup - */ - service->rate_limit.last=time(NULL) - USERS_REFRESH_TIME; - service->rate_limit.nloads=1; + if (service->users == NULL) { + /* + * Allocate specific data for MySQL users + * including hosts and db names + */ + service->users = mysql_users_alloc(); + + if ((loaded = load_mysql_users(service)) < 0) + { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Error : Unable to load users from %s:%d for " + "service %s.", + port->address, + port->port, + service->name))); + hashtable_free(service->users->data); + free(service->users); + dcb_free(port->listener); + port->listener = NULL; + goto retblock; + } + /* At service start last update is set to USERS_REFRESH_TIME seconds earlier. + * This way MaxScale could try reloading users' just after startup + */ + service->rate_limit.last=time(NULL) - USERS_REFRESH_TIME; + service->rate_limit.nloads=1; - LOGIF(LM, (skygw_log_write( - LOGFILE_MESSAGE, - "Loaded %d MySQL Users for service [%s].", - loaded, service->name))); + LOGIF(LM, (skygw_log_write( + LOGFILE_MESSAGE, + "Loaded %d MySQL Users for service [%s].", + loaded, service->name))); + } } else { - /* Generic users table */ - service->users = users_alloc(); + if (service->users == NULL) { + /* Generic users table */ + service->users = users_alloc(); + } } if ((funcs=(GWPROTOCOL *)load_module(port->protocol, MODULE_PROTOCOL)) From f466f866721b7ea22de419e174d2b98cc540aac3 Mon Sep 17 00:00:00 2001 From: MassimilianoPinto Date: Mon, 10 Nov 2014 11:10:32 +0100 Subject: [PATCH 2/4] Added default NULL for users table Added default NULL for users table --- server/core/service.c | 1 + 1 file changed, 1 insertion(+) diff --git a/server/core/service.c b/server/core/service.c index ae3c8edcd..d88013a37 100644 --- a/server/core/service.c +++ b/server/core/service.c @@ -145,6 +145,7 @@ SERVICE *service; service->filters = NULL; service->n_filters = 0; service->weightby = 0; + service->users = NULL; service->resources = NULL; spinlock_init(&service->spin); spinlock_init(&service->users_table_spin); From c2c12d8b00a1f604da1824e11d1cff12e50dca5d Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Mon, 10 Nov 2014 13:48:03 +0200 Subject: [PATCH 3/4] Added missing linker flags to connection test. --- server/modules/routing/test/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/modules/routing/test/CMakeLists.txt b/server/modules/routing/test/CMakeLists.txt index 550985243..62bdf06e8 100644 --- a/server/modules/routing/test/CMakeLists.txt +++ b/server/modules/routing/test/CMakeLists.txt @@ -1,6 +1,6 @@ if(MYSQLCLIENT_FOUND) add_executable(testconnect testconnect.c) message(STATUS "Linking against: ${MYSQLCLIENT_LIBRARIES}") - target_link_libraries(testconnect ${MYSQLCLIENT_LIBRARIES} ssl crypto dl z m) + target_link_libraries(testconnect ${MYSQLCLIENT_LIBRARIES} ssl crypto dl z m rt) add_test(NAME ReadConnRouterLoginTest COMMAND $ 10000 ${TEST_HOST} ${MASTER_PORT} ${TEST_HOST} ${TEST_PORT} 1.10) endif() \ No newline at end of file From e86b51865ce270700473a38be56db0a4e5c8c400 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Mon, 10 Nov 2014 13:51:21 +0200 Subject: [PATCH 4/4] Fixes to Coverity errors 72653, 72685, 72707, 73391, 73393, 73394, 73410 and 73414. --- server/modules/filter/hint/hintparser.c | 2 +- server/modules/filter/test/harness_common.c | 61 +++++++++++++------ server/modules/protocol/mysql_common.c | 38 +++++++++++- .../routing/readwritesplit/readwritesplit.c | 18 +++++- 4 files changed, 97 insertions(+), 22 deletions(-) diff --git a/server/modules/filter/hint/hintparser.c b/server/modules/filter/hint/hintparser.c index a70e42fe2..df0888508 100644 --- a/server/modules/filter/hint/hintparser.c +++ b/server/modules/filter/hint/hintparser.c @@ -550,7 +550,7 @@ HINT_TOKEN *tok; else if (!inword && inquote == '\0' && **ptr == '=') { *dest = **ptr; - *dest++; + dest++; (*ptr)++; break; } diff --git a/server/modules/filter/test/harness_common.c b/server/modules/filter/test/harness_common.c index 3c42bfdde..fe782bc0d 100644 --- a/server/modules/filter/test/harness_common.c +++ b/server/modules/filter/test/harness_common.c @@ -748,9 +748,12 @@ int load_filter(FILTERCHAIN* fc, CONFIG* cnf) } int x; - for(x = 0;xname); - free(fparams[x]->value); + + if(fparams){ + for(x = 0;xname); + free(fparams[x]->value); + } } free(fparams); @@ -769,15 +772,18 @@ FILTERCHAIN* load_filter_module(char* str) flt_ptr->next = instance.head; } - if((flt_ptr->instance = (FILTER_OBJECT*)load_module(str, MODULE_FILTER)) == NULL) - { - printf("Error: Module loading failed: %s\n",str); - skygw_log_write(LOGFILE_ERROR,"Error: Module loading failed: %s\n",str); - free(flt_ptr->down); - free(flt_ptr); - return NULL; - } - flt_ptr->name = strdup(str); + if(flt_ptr){ + if( (flt_ptr->instance = (FILTER_OBJECT*)load_module(str, MODULE_FILTER)) == NULL) + { + printf("Error: Module loading failed: %s\n",str); + skygw_log_write(LOGFILE_ERROR,"Error: Module loading failed: %s\n",str); + free(flt_ptr->down); + free(flt_ptr); + return NULL; + } + flt_ptr->name = strdup(str); + } + return flt_ptr; } @@ -925,14 +931,35 @@ GWBUF* gen_packet(PACKET pkt) int process_opts(int argc, char** argv) { - unsigned int fd = open_file("harness.cnf",1), buffsize = 1024; - int rd,rdsz; - unsigned int fsize; + int fd, buffsize = 1024; + int rd,rdsz, rval; + size_t fsize; char *buff = calloc(buffsize,sizeof(char)), *tok = NULL; /**Parse 'harness.cnf' file*/ - fsize = lseek(fd,0,SEEK_END); - lseek(fd,0,SEEK_SET); + + if(buff == NULL){ + printf("Error: Call to malloc() failed.\n"); + return 1; + } + + if((fd = open_file("harness.cnf",1)) < 0){ + printf("Failed to open configuration file.\n"); + free(buff); + return 1; + } + + + if( (rval = lseek(fd,0,SEEK_END)) < 0 || + lseek(fd,0,SEEK_SET) < 0){ + printf("Error: Cannot seek file.\n"); + close(fd); + free(buff); + return 1; + } + + fsize = (size_t)rval; + instance.thrcount = 1; instance.session_count = 1; rdsz = read(fd,buff,fsize); diff --git a/server/modules/protocol/mysql_common.c b/server/modules/protocol/mysql_common.c index 64cdd9f28..e54ea24ee 100644 --- a/server/modules/protocol/mysql_common.c +++ b/server/modules/protocol/mysql_common.c @@ -768,9 +768,43 @@ int gw_do_connect_to_backend( setipaddress(&serv_addr.sin_addr, host); serv_addr.sin_port = htons(port); bufsize = GW_BACKEND_SO_SNDBUF; - setsockopt(so, SOL_SOCKET, SO_SNDBUF, &bufsize, sizeof(bufsize)); + + if(setsockopt(so, SOL_SOCKET, SO_SNDBUF, &bufsize, sizeof(bufsize)) != 0) + { + int eno = errno; + errno = 0; + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Error: Failed to set socket options " + "%s:%d failed.\n\t\t Socket configuration failed " + "due %d, %s.", + host, + port, + eno, + strerror(eno)))); + rv = -1; + goto return_rv; + } + bufsize = GW_BACKEND_SO_RCVBUF; - setsockopt(so, SOL_SOCKET, SO_RCVBUF, &bufsize, sizeof(bufsize)); + + if(setsockopt(so, SOL_SOCKET, SO_RCVBUF, &bufsize, sizeof(bufsize)) != 0) + { + int eno = errno; + errno = 0; + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Error: Failed to set socket options " + "%s:%d failed.\n\t\t Socket configuration failed " + "due %d, %s.", + host, + port, + eno, + strerror(eno)))); + rv = -1; + goto return_rv; + } + /* set socket to as non-blocking here */ setnonblocking(so); rv = connect(so, (struct sockaddr *)&serv_addr, sizeof(serv_addr)); diff --git a/server/modules/routing/readwritesplit/readwritesplit.c b/server/modules/routing/readwritesplit/readwritesplit.c index d7f68b5c3..574823660 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.c +++ b/server/modules/routing/readwritesplit/readwritesplit.c @@ -830,8 +830,16 @@ static void* newSession( * Find a backend servers to connect to. * This command requires that rsession's lock is held. */ - rses_begin_locked_router_action(client_rses); + succp = rses_begin_locked_router_action(client_rses); + + if(!succp){ + free(client_rses->rses_backend_ref); + free(client_rses); + client_rses = NULL; + goto return_rses; + } + succp = select_connect_backend_servers(&master_ref, backend_ref, router_nservers, @@ -1610,8 +1618,12 @@ void check_create_tmp_table( rses_prop_tmp->rses_prop_type = RSES_PROP_TYPE_TMPTABLES; router_cli_ses->rses_properties[RSES_PROP_TYPE_TMPTABLES] = rses_prop_tmp; } + else + { + LOGIF(LE, (skygw_log_write_flush(LOGFILE_ERROR,"Error : Call to malloc() failed."))); + } } - + if(rses_prop_tmp){ if (rses_prop_tmp->rses_prop_data.temp_tables == NULL) { h = hashtable_alloc(7, hashkeyfun, hashcmpfun); @@ -1647,6 +1659,8 @@ void check_create_tmp_table( } } #endif + } + free(hkey); free(tblname); }