From b4cb252f91d233c9262bb18acbdc1e65b035b9e8 Mon Sep 17 00:00:00 2001 From: Hartmut Holzgraefe Date: Fri, 13 Feb 2015 16:10:21 +0100 Subject: [PATCH 01/18] ignore warnings about unused functions and variables for now --- CMakeLists.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 8d8023318..aa8721e64 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -40,8 +40,8 @@ configure_file(${CMAKE_SOURCE_DIR}/server/test/maxscale_test.h.in ${CMAKE_BINARY configure_file(${CMAKE_SOURCE_DIR}/etc/postinst.in ${CMAKE_BINARY_DIR}/postinst) configure_file(${CMAKE_SOURCE_DIR}/etc/postrm.in ${CMAKE_BINARY_DIR}/postrm) -set(CMAKE_C_FLAGS "-Wall -fPIC") -set(CMAKE_CXX_FLAGS "-Wall -fPIC") +set(CMAKE_C_FLAGS "-Wall -Wno-unused-variable -Wno-unused-but-set-variable -Wno-unused-function -fPIC") +set(CMAKE_CXX_FLAGS "-Wall -Wno-unused-variable -Wno-unused-but-set-variable -fPIC") set(DEBUG_FLAGS "-ggdb -pthread -pipe -Wformat -fstack-protector --param=ssp-buffer-size=4") if(CMAKE_VERSION VERSION_GREATER 2.6) From be995a1ac65cb40adf5b68b533cc5b248ddc8021 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Fri, 13 Feb 2015 17:19:28 +0200 Subject: [PATCH 02/18] Changed strtok to strtok_r in fwfilter.c --- server/modules/filter/fwfilter.c | 48 +++++++++++++++++--------------- 1 file changed, 25 insertions(+), 23 deletions(-) diff --git a/server/modules/filter/fwfilter.c b/server/modules/filter/fwfilter.c index 34fd4e17f..31f286a9e 100644 --- a/server/modules/filter/fwfilter.c +++ b/server/modules/filter/fwfilter.c @@ -639,6 +639,7 @@ void link_rules(char* rule, FW_INSTANCE* instance) bool match_any; char *tok, *ruleptr, *userptr, *modeptr; + char **saveptr; RULELIST* rulelist = NULL; userptr = strstr(rule,"users "); @@ -653,10 +654,10 @@ void link_rules(char* rule, FW_INSTANCE* instance) *modeptr++ = '\0'; *ruleptr++ = '\0'; - - tok = strtok(modeptr," "); + + tok = strtok_r(modeptr," ",saveptr); if(strcmp(tok,"match") == 0){ - tok = strtok(NULL," "); + tok = strtok_r(NULL," ",saveptr); if(strcmp(tok,"any") == 0){ match_any = true; }else if(strcmp(tok,"all") == 0){ @@ -667,8 +668,8 @@ void link_rules(char* rule, FW_INSTANCE* instance) } } - tok = strtok(ruleptr," "); - tok = strtok(NULL," "); + tok = strtok_r(ruleptr," ",saveptr); + tok = strtok_r(NULL," ",saveptr); while(tok) { @@ -682,7 +683,7 @@ void link_rules(char* rule, FW_INSTANCE* instance) rulelist = tmp_rl; } - tok = strtok(NULL," "); + tok = strtok_r(NULL," ",saveptr); } /** @@ -690,8 +691,8 @@ void link_rules(char* rule, FW_INSTANCE* instance) */ *(ruleptr) = '\0'; - userptr = strtok(rule," "); - userptr = strtok(NULL," "); + userptr = strtok_r(rule," ",saveptr); + userptr = strtok_r(NULL," ",saveptr); while(userptr) { @@ -738,7 +739,7 @@ void link_rules(char* rule, FW_INSTANCE* instance) (void *)userptr, (void *)user); - userptr = strtok(NULL," "); + userptr = strtok_r(NULL," ",saveptr); } @@ -755,7 +756,8 @@ void parse_rule(char* rule, FW_INSTANCE* instance) ss_dassert(rule != NULL && instance != NULL); char *rulecpy = strdup(rule); - char *tok = strtok(rulecpy," ,"); + char **saveptr; + char *tok = strtok_r(rulecpy," ,",saveptr); bool allow,deny,mode; RULE* ruledef = NULL; @@ -763,7 +765,7 @@ void parse_rule(char* rule, FW_INSTANCE* instance) if(strcmp("rule",tok) == 0){ /**Define a new rule*/ - tok = strtok(NULL," ,"); + tok = strtok_r(NULL," ,",saveptr); if(tok == NULL) goto retblock; @@ -799,7 +801,7 @@ void parse_rule(char* rule, FW_INSTANCE* instance) goto retblock; } - tok = strtok(NULL, " ,"); + tok = strtok_r(NULL, " ,",saveptr); if((allow = (strcmp(tok,"allow") == 0)) || @@ -808,7 +810,7 @@ void parse_rule(char* rule, FW_INSTANCE* instance) mode = allow ? true:false; ruledef->allow = mode; ruledef->type = RT_PERMISSION; - tok = strtok(NULL, " ,"); + tok = strtok_r(NULL, " ,",saveptr); while(tok){ @@ -820,13 +822,13 @@ void parse_rule(char* rule, FW_INSTANCE* instance) { STRLINK *tail = NULL,*current; ruledef->type = RT_COLUMN; - tok = strtok(NULL, " ,"); + tok = strtok_r(NULL, " ,",saveptr); while(tok && strcmp(tok,"at_times") != 0){ current = malloc(sizeof(STRLINK)); current->value = strdup(tok); current->next = tail; tail = current; - tok = strtok(NULL, " ,"); + tok = strtok_r(NULL, " ,",saveptr); } ruledef->data = (void*)tail; @@ -836,7 +838,7 @@ void parse_rule(char* rule, FW_INSTANCE* instance) else if(strcmp(tok,"at_times") == 0) { - tok = strtok(NULL, " ,"); + tok = strtok_r(NULL, " ,",saveptr); TIMERANGE *tr = NULL; while(tok){ TIMERANGE *tmp = parse_time(tok,instance); @@ -846,7 +848,7 @@ void parse_rule(char* rule, FW_INSTANCE* instance) } tmp->next = tr; tr = tmp; - tok = strtok(NULL, " ,"); + tok = strtok_r(NULL, " ,",saveptr); } ruledef->active = tr; } @@ -855,7 +857,7 @@ void parse_rule(char* rule, FW_INSTANCE* instance) bool escaped = false; regex_t *re; char* start, *str; - tok = strtok(NULL," "); + tok = strtok_r(NULL," ",saveptr); char delim = '\''; while(*tok == '\'' || *tok == '"'){ delim = *tok; @@ -914,20 +916,20 @@ void parse_rule(char* rule, FW_INSTANCE* instance) qs->id = ++instance->idgen; spinlock_release(instance->lock); - tok = strtok(NULL," "); + tok = strtok_r(NULL," ",saveptr); if(tok == NULL){ free(qs); goto retblock; } qs->limit = atoi(tok); - tok = strtok(NULL," "); + tok = strtok_r(NULL," ",saveptr); if(tok == NULL){ free(qs); goto retblock; } qs->period = atof(tok); - tok = strtok(NULL," "); + tok = strtok_r(NULL," ",saveptr); if(tok == NULL){ free(qs); goto retblock; @@ -943,7 +945,7 @@ void parse_rule(char* rule, FW_INSTANCE* instance) } else if(strcmp(tok,"on_operations") == 0) { - tok = strtok(NULL," "); + tok = strtok_r(NULL," ",saveptr); if(!parse_querytypes(tok,ruledef)){ skygw_log_write(LOGFILE_ERROR, "fwfilter: Invalid query type" @@ -951,7 +953,7 @@ void parse_rule(char* rule, FW_INSTANCE* instance) ,tok); } } - tok = strtok(NULL," ,"); + tok = strtok_r(NULL," ,",saveptr); } goto retblock; From 792d4e9c8ca3be76f73836a958c5211105d893c6 Mon Sep 17 00:00:00 2001 From: Hartmut Holzgraefe Date: Fri, 13 Feb 2015 16:28:57 +0100 Subject: [PATCH 03/18] fix 'unused value' warning --- query_classifier/query_classifier.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/query_classifier/query_classifier.cc b/query_classifier/query_classifier.cc index 874251d32..892e95eb9 100644 --- a/query_classifier/query_classifier.cc +++ b/query_classifier/query_classifier.cc @@ -1253,7 +1253,7 @@ char* skygw_get_affected_fields(GWBUF* buf) List_iterator ilist(lex->current_select->item_list); item = (Item*)ilist.next(); - for (item; item != NULL; item=(Item*)ilist.next()) + for (; item != NULL; item=(Item*)ilist.next()) { itype = item->type(); From a3a7aae1d2cb39b6d8436d4b4b389f21374ebea0 Mon Sep 17 00:00:00 2001 From: Hartmut Holzgraefe Date: Fri, 13 Feb 2015 16:57:12 +0100 Subject: [PATCH 04/18] add missing return type for main() --- server/core/maxkeys.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/core/maxkeys.c b/server/core/maxkeys.c index ad1e28f86..747e575ec 100644 --- a/server/core/maxkeys.c +++ b/server/core/maxkeys.c @@ -30,7 +30,7 @@ #include #include -main(int argc, char **argv) +int main(int argc, char **argv) { if (argc != 2) { From c992b53750ba1e6e7a2bf0217a90891071986719 Mon Sep 17 00:00:00 2001 From: Hartmut Holzgraefe Date: Fri, 13 Feb 2015 17:03:39 +0100 Subject: [PATCH 05/18] include stdlib.h for malloc() prototype --- server/core/memlog.c | 1 + 1 file changed, 1 insertion(+) diff --git a/server/core/memlog.c b/server/core/memlog.c index 43df8c5da..fc26fc7e7 100644 --- a/server/core/memlog.c +++ b/server/core/memlog.c @@ -28,6 +28,7 @@ * @endverbatim */ #include +#include #include static MEMLOG *memlogs = NULL; From 56ab0efbb3853c63369fdffa2586726e6c5e27e8 Mon Sep 17 00:00:00 2001 From: Hartmut Holzgraefe Date: Fri, 13 Feb 2015 17:06:47 +0100 Subject: [PATCH 06/18] include string.h for strdup() prototype --- server/core/memlog.c | 1 + 1 file changed, 1 insertion(+) diff --git a/server/core/memlog.c b/server/core/memlog.c index fc26fc7e7..95d23bb91 100644 --- a/server/core/memlog.c +++ b/server/core/memlog.c @@ -30,6 +30,7 @@ #include #include #include +#include static MEMLOG *memlogs = NULL; static SPINLOCK *memlock = SPINLOCK_INIT; From 216688338387dadf86f7c11ded6a73cb7c6fc6cd Mon Sep 17 00:00:00 2001 From: Hartmut Holzgraefe Date: Fri, 13 Feb 2015 17:23:25 +0100 Subject: [PATCH 07/18] consistently use size_t for buffer and string length calculations, not int --- log_manager/log_manager.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/log_manager/log_manager.cc b/log_manager/log_manager.cc index 04eef42e4..79d739af3 100644 --- a/log_manager/log_manager.cc +++ b/log_manager/log_manager.cc @@ -167,7 +167,7 @@ struct logfile_st { size_t lf_file_size; /** list of block-sized log buffers */ mlist_t lf_blockbuf_list; - int lf_buf_size; + size_t lf_buf_size; bool lf_flushflag; bool lf_rotateflag; int lf_spinlock; /**< lf_flushflag & lf_rotateflag */ @@ -633,7 +633,7 @@ static int logmanager_write_log( int err = 0; blockbuf_t* bb; blockbuf_t* bb_c; - int timestamp_len; + size_t timestamp_len; int i; CHK_LOGMANAGER(lm); @@ -680,9 +680,9 @@ static int logmanager_write_log( else { /** Length of string that will be written, limited by bufsize */ - int safe_str_len; + size_t safe_str_len; /** Length of session id */ - int sesid_str_len; + size_t sesid_str_len; /** * 2 braces, 2 spaces and terminating char From 21613cb03d0b5a45cfba4f23bdf1a2b624cd0336 Mon Sep 17 00:00:00 2001 From: Hartmut Holzgraefe Date: Fri, 13 Feb 2015 17:30:02 +0100 Subject: [PATCH 08/18] fix comparison prototype to match qsort() expectation --- server/modules/filter/topfilter.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/server/modules/filter/topfilter.c b/server/modules/filter/topfilter.c index 71a6d86ca..737dd790a 100644 --- a/server/modules/filter/topfilter.c +++ b/server/modules/filter/topfilter.c @@ -488,8 +488,11 @@ char *ptr; } static int -cmp_topn(TOPNQ **a, TOPNQ **b) +cmp_topn(const void *va, const void *vb) { + TOPNQ **a = (TOPNQ **)va; + TOPNQ **b = (TOPNQ **)vb; + if ((*b)->duration.tv_sec == (*a)->duration.tv_sec) return (*b)->duration.tv_usec - (*a)->duration.tv_usec; return (*b)->duration.tv_sec - (*a)->duration.tv_sec; From 2477043d11bde7a2dd28ff9c5535e0b121ff8105 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Fri, 13 Feb 2015 23:59:32 +0200 Subject: [PATCH 09/18] Logfiles are now writted to file instead of shared memory as a default. This can still be enabled while starting maxscale. --- server/core/gateway.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/core/gateway.c b/server/core/gateway.c index f5bc87cc1..10dbbfc31 100644 --- a/server/core/gateway.c +++ b/server/core/gateway.c @@ -991,7 +991,7 @@ static void usage(void) " -f|--config=... relative|absolute pathname of MaxScale configuration file\n" " (default: $MAXSCALE_HOME/etc/MaxScale.cnf)\n" " -l|--log=... log to file or shared memory\n" - " -lfile or -lshm - defaults to shared memory\n" + " -lfile or -lshm - defaults to file\n" " -v|--version print version info and exit\n" " -?|--help show this help\n" , progname); @@ -1052,7 +1052,7 @@ int main(int argc, char **argv) char* cnf_file_arg = NULL; /*< conf filename from cmd-line arg */ void* log_flush_thr = NULL; int option_index; - int logtofile = 0; /* Use shared memory or file */ + int logtofile = 1; /* Use shared memory or file */ ssize_t log_flush_timeout_ms = 0; sigset_t sigset; sigset_t sigpipe_mask; From 3bad5dc8144a8e30a1f28aac4714f55dcefe72e0 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Sat, 14 Feb 2015 07:54:17 +0200 Subject: [PATCH 10/18] Fixes to Coverity defects 87308, 87307, 87306, 87074, 87068. --- query_classifier/query_classifier.cc | 2 +- server/core/hashtable.c | 11 ++++++++--- server/modules/filter/fwfilter.c | 19 +++++++++++++++---- 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/query_classifier/query_classifier.cc b/query_classifier/query_classifier.cc index 874251d32..9667193ed 100644 --- a/query_classifier/query_classifier.cc +++ b/query_classifier/query_classifier.cc @@ -1572,7 +1572,7 @@ char* skygw_get_qtype_str( skygw_query_op_t query_classifier_get_operation(GWBUF* querybuf) { LEX* lex = get_lex(querybuf); - skygw_query_op_t operation; + skygw_query_op_t operation = QUERY_OP_UNDEFINED; if(lex){ switch(lex->sql_command){ case SQLCOM_SELECT: diff --git a/server/core/hashtable.c b/server/core/hashtable.c index 86d1d821e..df88ac9f7 100644 --- a/server/core/hashtable.c +++ b/server/core/hashtable.c @@ -678,12 +678,14 @@ void *key, *value; if (!(*keywrite)(fd, key)) { close(fd); + hashtable_iterator_free(iter); return -1; } if ((value = hashtable_fetch(table, key)) == NULL || (*valuewrite)(fd, value) == 0) { close(fd); + hashtable_iterator_free(iter); return -1; } rval++; @@ -691,10 +693,13 @@ void *key, *value; } /* Now go back and write the count of entries */ - lseek(fd, 7L, SEEK_SET); - write(fd, &rval, sizeof(rval)); - + if(lseek(fd, 7L, SEEK_SET) != -1) + { + write(fd, &rval, sizeof(rval)); + } + close(fd); + hashtable_iterator_free(iter); return rval; } diff --git a/server/modules/filter/fwfilter.c b/server/modules/filter/fwfilter.c index 31f286a9e..5cd8d5d78 100644 --- a/server/modules/filter/fwfilter.c +++ b/server/modules/filter/fwfilter.c @@ -639,7 +639,7 @@ void link_rules(char* rule, FW_INSTANCE* instance) bool match_any; char *tok, *ruleptr, *userptr, *modeptr; - char **saveptr; + char **saveptr = NULL; RULELIST* rulelist = NULL; userptr = strstr(rule,"users "); @@ -656,7 +656,7 @@ void link_rules(char* rule, FW_INSTANCE* instance) *ruleptr++ = '\0'; tok = strtok_r(modeptr," ",saveptr); - if(strcmp(tok,"match") == 0){ + if(tok && strcmp(tok,"match") == 0){ tok = strtok_r(NULL," ",saveptr); if(strcmp(tok,"any") == 0){ match_any = true; @@ -742,7 +742,13 @@ void link_rules(char* rule, FW_INSTANCE* instance) userptr = strtok_r(NULL," ",saveptr); } - + + while(rulelist) + { + RULELIST *tmp = rulelist; + rulelist = rulelist->next; + free(tmp); + } } @@ -756,7 +762,7 @@ void parse_rule(char* rule, FW_INSTANCE* instance) ss_dassert(rule != NULL && instance != NULL); char *rulecpy = strdup(rule); - char **saveptr; + char **saveptr = NULL; char *tok = strtok_r(rulecpy," ,",saveptr); bool allow,deny,mode; RULE* ruledef = NULL; @@ -800,6 +806,11 @@ void parse_rule(char* rule, FW_INSTANCE* instance) add_users(rule, instance); goto retblock; } + else + { + skygw_log_write(LOGFILE_ERROR,"Error : Unknown token in rule file: %s",tok); + goto retblock; + } tok = strtok_r(NULL, " ,",saveptr); From 3ecf9260923848d74293e850ae8cf03da7e126d1 Mon Sep 17 00:00:00 2001 From: Hartmut Holzgraefe Date: Sat, 14 Feb 2015 15:34:11 +0100 Subject: [PATCH 11/18] prevent signed/unsigned conflicts --- server/core/modutil.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/core/modutil.c b/server/core/modutil.c index 07ef9af85..269c66635 100644 --- a/server/core/modutil.c +++ b/server/core/modutil.c @@ -237,7 +237,7 @@ char * modutil_get_SQL(GWBUF *buf) { unsigned int len, length; -unsigned char *ptr, *dptr, *rval = NULL; +char *ptr, *dptr, *rval = NULL; if (!modutil_is_SQL(buf)) return rval; @@ -671,4 +671,4 @@ static void modutil_reply_routing_error( /** Create an incoming event for backend DCB */ poll_add_epollin_event_to_dcb(backend_dcb, buf); return; -} \ No newline at end of file +} From cb35472133bcc8e4d104811c9d533b129fd27a18 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Sat, 14 Feb 2015 17:38:05 +0200 Subject: [PATCH 12/18] Fixed wrong type of char pointer in strtok_r calls in fwfilter.c --- server/modules/filter/fwfilter.c | 48 +++++++++++------------ server/modules/filter/test/CMakeLists.txt | 1 + 2 files changed, 25 insertions(+), 24 deletions(-) diff --git a/server/modules/filter/fwfilter.c b/server/modules/filter/fwfilter.c index 5cd8d5d78..595be17ea 100644 --- a/server/modules/filter/fwfilter.c +++ b/server/modules/filter/fwfilter.c @@ -639,7 +639,7 @@ void link_rules(char* rule, FW_INSTANCE* instance) bool match_any; char *tok, *ruleptr, *userptr, *modeptr; - char **saveptr = NULL; + char *saveptr = NULL; RULELIST* rulelist = NULL; userptr = strstr(rule,"users "); @@ -655,9 +655,9 @@ void link_rules(char* rule, FW_INSTANCE* instance) *modeptr++ = '\0'; *ruleptr++ = '\0'; - tok = strtok_r(modeptr," ",saveptr); + tok = strtok_r(modeptr," ",&saveptr); if(tok && strcmp(tok,"match") == 0){ - tok = strtok_r(NULL," ",saveptr); + tok = strtok_r(NULL," ",&saveptr); if(strcmp(tok,"any") == 0){ match_any = true; }else if(strcmp(tok,"all") == 0){ @@ -668,8 +668,8 @@ void link_rules(char* rule, FW_INSTANCE* instance) } } - tok = strtok_r(ruleptr," ",saveptr); - tok = strtok_r(NULL," ",saveptr); + tok = strtok_r(ruleptr," ",&saveptr); + tok = strtok_r(NULL," ",&saveptr); while(tok) { @@ -683,7 +683,7 @@ void link_rules(char* rule, FW_INSTANCE* instance) rulelist = tmp_rl; } - tok = strtok_r(NULL," ",saveptr); + tok = strtok_r(NULL," ",&saveptr); } /** @@ -691,8 +691,8 @@ void link_rules(char* rule, FW_INSTANCE* instance) */ *(ruleptr) = '\0'; - userptr = strtok_r(rule," ",saveptr); - userptr = strtok_r(NULL," ",saveptr); + userptr = strtok_r(rule," ",&saveptr); + userptr = strtok_r(NULL," ",&saveptr); while(userptr) { @@ -739,7 +739,7 @@ void link_rules(char* rule, FW_INSTANCE* instance) (void *)userptr, (void *)user); - userptr = strtok_r(NULL," ",saveptr); + userptr = strtok_r(NULL," ",&saveptr); } @@ -762,8 +762,8 @@ void parse_rule(char* rule, FW_INSTANCE* instance) ss_dassert(rule != NULL && instance != NULL); char *rulecpy = strdup(rule); - char **saveptr = NULL; - char *tok = strtok_r(rulecpy," ,",saveptr); + char *saveptr = NULL; + char *tok = strtok_r(rulecpy," ,",&saveptr); bool allow,deny,mode; RULE* ruledef = NULL; @@ -771,7 +771,7 @@ void parse_rule(char* rule, FW_INSTANCE* instance) if(strcmp("rule",tok) == 0){ /**Define a new rule*/ - tok = strtok_r(NULL," ,",saveptr); + tok = strtok_r(NULL," ,",&saveptr); if(tok == NULL) goto retblock; @@ -812,7 +812,7 @@ void parse_rule(char* rule, FW_INSTANCE* instance) goto retblock; } - tok = strtok_r(NULL, " ,",saveptr); + tok = strtok_r(NULL, " ,",&saveptr); if((allow = (strcmp(tok,"allow") == 0)) || @@ -821,7 +821,7 @@ void parse_rule(char* rule, FW_INSTANCE* instance) mode = allow ? true:false; ruledef->allow = mode; ruledef->type = RT_PERMISSION; - tok = strtok_r(NULL, " ,",saveptr); + tok = strtok_r(NULL, " ,",&saveptr); while(tok){ @@ -833,13 +833,13 @@ void parse_rule(char* rule, FW_INSTANCE* instance) { STRLINK *tail = NULL,*current; ruledef->type = RT_COLUMN; - tok = strtok_r(NULL, " ,",saveptr); + tok = strtok_r(NULL, " ,",&saveptr); while(tok && strcmp(tok,"at_times") != 0){ current = malloc(sizeof(STRLINK)); current->value = strdup(tok); current->next = tail; tail = current; - tok = strtok_r(NULL, " ,",saveptr); + tok = strtok_r(NULL, " ,",&saveptr); } ruledef->data = (void*)tail; @@ -849,7 +849,7 @@ void parse_rule(char* rule, FW_INSTANCE* instance) else if(strcmp(tok,"at_times") == 0) { - tok = strtok_r(NULL, " ,",saveptr); + tok = strtok_r(NULL, " ,",&saveptr); TIMERANGE *tr = NULL; while(tok){ TIMERANGE *tmp = parse_time(tok,instance); @@ -859,7 +859,7 @@ void parse_rule(char* rule, FW_INSTANCE* instance) } tmp->next = tr; tr = tmp; - tok = strtok_r(NULL, " ,",saveptr); + tok = strtok_r(NULL, " ,",&saveptr); } ruledef->active = tr; } @@ -868,7 +868,7 @@ void parse_rule(char* rule, FW_INSTANCE* instance) bool escaped = false; regex_t *re; char* start, *str; - tok = strtok_r(NULL," ",saveptr); + tok = strtok_r(NULL," ",&saveptr); char delim = '\''; while(*tok == '\'' || *tok == '"'){ delim = *tok; @@ -927,20 +927,20 @@ void parse_rule(char* rule, FW_INSTANCE* instance) qs->id = ++instance->idgen; spinlock_release(instance->lock); - tok = strtok_r(NULL," ",saveptr); + tok = strtok_r(NULL," ",&saveptr); if(tok == NULL){ free(qs); goto retblock; } qs->limit = atoi(tok); - tok = strtok_r(NULL," ",saveptr); + tok = strtok_r(NULL," ",&saveptr); if(tok == NULL){ free(qs); goto retblock; } qs->period = atof(tok); - tok = strtok_r(NULL," ",saveptr); + tok = strtok_r(NULL," ",&saveptr); if(tok == NULL){ free(qs); goto retblock; @@ -956,7 +956,7 @@ void parse_rule(char* rule, FW_INSTANCE* instance) } else if(strcmp(tok,"on_operations") == 0) { - tok = strtok_r(NULL," ",saveptr); + tok = strtok_r(NULL," ",&saveptr); if(!parse_querytypes(tok,ruledef)){ skygw_log_write(LOGFILE_ERROR, "fwfilter: Invalid query type" @@ -964,7 +964,7 @@ void parse_rule(char* rule, FW_INSTANCE* instance) ,tok); } } - tok = strtok_r(NULL," ,",saveptr); + tok = strtok_r(NULL," ,",&saveptr); } goto retblock; diff --git a/server/modules/filter/test/CMakeLists.txt b/server/modules/filter/test/CMakeLists.txt index ed1f18e7e..2ad0c2201 100644 --- a/server/modules/filter/test/CMakeLists.txt +++ b/server/modules/filter/test/CMakeLists.txt @@ -11,6 +11,7 @@ add_executable(harness harness_util.c harness_common.c ${CORE}) target_link_libraries(harness_ui fullcore log_manager utils) target_link_libraries(harness fullcore) execute_process(COMMAND ${CMAKE_COMMAND} -E copy ${ERRMSG} ${CMAKE_CURRENT_BINARY_DIR}) +execute_process(COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_SOURCE_DIR}/harness.cnf ${CMAKE_CURRENT_BINARY_DIR}) configure_file(${CMAKE_CURRENT_SOURCE_DIR}/testdriver.sh ${CMAKE_CURRENT_BINARY_DIR}/testdriver.sh @ONLY) configure_file(${CMAKE_CURRENT_SOURCE_DIR}/hintfilter/hint_testing.cnf ${CMAKE_CURRENT_BINARY_DIR}/hintfilter/hint_testing.cnf) From 617f44c9c98221648201b21745d4779b0a2bc78f Mon Sep 17 00:00:00 2001 From: Hartmut Holzgraefe Date: Sat, 14 Feb 2015 16:39:05 +0100 Subject: [PATCH 13/18] use intptr_t type for pointer->integer conversion so that int <-> pointer sizes match --- server/core/gateway.c | 9 +++++---- server/core/poll.c | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/server/core/gateway.c b/server/core/gateway.c index f5bc87cc1..99673d751 100644 --- a/server/core/gateway.c +++ b/server/core/gateway.c @@ -1039,6 +1039,7 @@ int main(int argc, char **argv) int l; int i; int n; + intptr_t thread_id; int n_threads; /*< number of epoll listener threads */ int n_services; int eno = 0; /*< local variable for errno */ @@ -1788,9 +1789,9 @@ int main(int argc, char **argv) /*< * Start server threads. */ - for (n = 0; n < n_threads - 1; n++) + for (thread_id = 0; thread_id < n_threads - 1; thread_id++) { - threads[n] = thread_start(poll_waitevents, (void *)(n + 1)); + threads[thread_id] = thread_start(poll_waitevents, (void *)(thread_id + 1)); } LOGIF(LM, (skygw_log_write(LOGFILE_MESSAGE, "MaxScale started with %d server threads.", @@ -1803,9 +1804,9 @@ int main(int argc, char **argv) /*< * Wait server threads' completion. */ - for (n = 0; n < n_threads - 1; n++) + for (thread_id = 0; thread_id < n_threads - 1; thread_id++) { - thread_wait(threads[n]); + thread_wait(threads[thread_id]); } /*< * Wait the flush thread. diff --git a/server/core/poll.c b/server/core/poll.c index 2dc4c34ae..46d049592 100644 --- a/server/core/poll.c +++ b/server/core/poll.c @@ -438,7 +438,7 @@ poll_waitevents(void *arg) { struct epoll_event events[MAX_EVENTS]; int i, nfds, timeout_bias = 1; -int thread_id = (int)arg; +intptr_t thread_id = (intptr_t)arg; DCB *zombies = NULL; int poll_spins = 0; From eedcd44e954a54f869467db318f347ea68d43e6c Mon Sep 17 00:00:00 2001 From: Hartmut Holzgraefe Date: Sat, 14 Feb 2015 16:51:03 +0100 Subject: [PATCH 14/18] wrong SPINLOCK type declaration (actual structure, not pointer, needed) --- server/core/memlog.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/core/memlog.c b/server/core/memlog.c index 95d23bb91..8e44e2d5c 100644 --- a/server/core/memlog.c +++ b/server/core/memlog.c @@ -33,7 +33,7 @@ #include static MEMLOG *memlogs = NULL; -static SPINLOCK *memlock = SPINLOCK_INIT; +static SPINLOCK memlock = SPINLOCK_INIT; /** * Create a new instance of a memory logger. From 691eefe0dea3793271d956e29ed310e10ce8ca78 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Sun, 15 Feb 2015 19:51:40 +0200 Subject: [PATCH 15/18] Fixed log manager writing to the file when it checks that they exist. --- log_manager/log_manager.cc | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/log_manager/log_manager.cc b/log_manager/log_manager.cc index 04eef42e4..c9826b08a 100644 --- a/log_manager/log_manager.cc +++ b/log_manager/log_manager.cc @@ -2356,7 +2356,7 @@ static bool check_file_and_path( /** File exists, check permission to read/write */ if (errno == EEXIST) { - /** Open file and write a byte for test */ + /** Open file */ fd = open(filename, O_CREAT|O_RDWR, S_IRWXU|S_IRWXG); if (fd == -1) @@ -2387,8 +2387,7 @@ static bool check_file_and_path( { if (writable) { - char c = ' '; - if (write(fd, &c, 1) == 1) + if (access(filename,W_OK) == 0) { *writable = true; } From 678fbb4646b3330419c195a828d9f215fc09358e Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Sun, 15 Feb 2015 20:10:09 +0200 Subject: [PATCH 16/18] Log manager no longer creates files when checking if they exist. --- log_manager/log_manager.cc | 154 ++++++++++++------------------------- 1 file changed, 48 insertions(+), 106 deletions(-) diff --git a/log_manager/log_manager.cc b/log_manager/log_manager.cc index c9826b08a..ff2b3e695 100644 --- a/log_manager/log_manager.cc +++ b/log_manager/log_manager.cc @@ -2335,7 +2335,6 @@ static bool check_file_and_path( bool* writable, bool do_log) { - int fd; bool exists; if (filename == NULL) @@ -2349,111 +2348,54 @@ static bool check_file_and_path( } else { - fd = open(filename, O_CREAT|O_EXCL, S_IRWXU); - - if (fd == -1) - { - /** File exists, check permission to read/write */ - if (errno == EEXIST) - { - /** Open file */ - fd = open(filename, O_CREAT|O_RDWR, S_IRWXU|S_IRWXG); - - if (fd == -1) - { - if (do_log && file_is_symlink(filename)) - { - fprintf(stderr, - "*\n* Error : Can't access " - "file pointed to by %s due " - "to %s.\n", - filename, - strerror(errno)); - } - else if (do_log) - { - fprintf(stderr, - "*\n* Error : Can't access %s due " - "to %s.\n", - filename, - strerror(errno)); - } - if (writable) - { - *writable = false; - } - } - else - { - if (writable) - { - if (access(filename,W_OK) == 0) - { - *writable = true; - } - else - { - if (do_log && - file_is_symlink(filename)) - { - fprintf(stderr, - "*\n* Error : Can't write to " - "file pointed to by %s due to " - "%s.\n", - filename, - strerror(errno)); - } - else if (do_log) - { - fprintf(stderr, - "*\n* Error : Can't write to " - "%s due to %s.\n", - filename, - strerror(errno)); - } - *writable = false; - } - } - close(fd); - } - exists = true; - } - else - { - if (do_log && file_is_symlink(filename)) - { - fprintf(stderr, - "*\n* Error : Can't access the file " - "pointed to by %s due to %s.\n", - filename, - strerror(errno)); - } - else if (do_log) - { - fprintf(stderr, - "*\n* Error : Can't access %s due to %s.\n", - filename, - strerror(errno)); - } - exists = false; - - if (writable) - { - *writable = false; - } - } - } - else - { - close(fd); - unlink(filename); - exists = false; - - if (writable) - { - *writable = true; - } - } + if(access(filename,F_OK) == 0) + { + + exists = true; + + if(access(filename,W_OK) == 0) + { + if(writable) + { + *writable = true; + } + } + else + { + + if (do_log && file_is_symlink(filename)) + { + fprintf(stderr, + "*\n* Error : Can't access " + "file pointed to by %s due " + "to %s.\n", + filename, + strerror(errno)); + } + else if (do_log) + { + fprintf(stderr, + "*\n* Error : Can't access %s due " + "to %s.\n", + filename, + strerror(errno)); + } + + if(writable) + { + *writable = false; + } + } + + } + else + { + exists = false; + if(writable) + { + *writable = true; + } + } } return exists; } From cd99d6c1dd45379e69b25cc167c0456df32cc19b Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Mon, 16 Feb 2015 09:11:55 +0200 Subject: [PATCH 17/18] Fixes to Coverity defects 87073, 87388. --- server/core/service.c | 30 +++++++++++++++++++++++++++--- server/modules/filter/fwfilter.c | 2 +- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/server/core/service.c b/server/core/service.c index 95f9ac771..167f8a4e7 100644 --- a/server/core/service.c +++ b/server/core/service.c @@ -221,7 +221,7 @@ GWPROTOCOL *funcs; { /* Try loading authentication data from file cache */ - char *ptr, path[4096]; + char *ptr, path[4097]; strcpy(path, "/usr/local/skysql/MaxScale"); if ((ptr = getenv("MAXSCALE_HOME")) != NULL) { @@ -251,6 +251,7 @@ GWPROTOCOL *funcs; { /* Save authentication data to file cache */ char *ptr, path[4096]; + int mkdir_rval = 0; strcpy(path, "/usr/local/skysql/MaxScale"); if ((ptr = getenv("MAXSCALE_HOME")) != NULL) { @@ -259,10 +260,33 @@ GWPROTOCOL *funcs; strncat(path, "/", 4096); strncat(path, service->name, 4096); if (access(path, R_OK) == -1) - mkdir(path, 0777); + { + mkdir_rval = mkdir(path, 0777); + } + + if(mkdir_rval) + { + skygw_log_write(LOGFILE_ERROR,"Error : Failed to create directory '%s': [%d] %s", + path, + errno, + strerror(errno)); + mkdir_rval = 0; + } + strncat(path, "/.cache", 4096); if (access(path, R_OK) == -1) - mkdir(path, 0777); + { + mkdir_rval = mkdir(path, 0777); + } + + if(mkdir_rval) + { + skygw_log_write(LOGFILE_ERROR,"Error : Failed to create directory '%s': [%d] %s", + path, + errno, + strerror(errno)); + mkdir_rval = 0; + } strncat(path, "/dbusers", 4096); dbusers_save(service->users, path); } diff --git a/server/modules/filter/fwfilter.c b/server/modules/filter/fwfilter.c index 595be17ea..7766521cf 100644 --- a/server/modules/filter/fwfilter.c +++ b/server/modules/filter/fwfilter.c @@ -637,7 +637,7 @@ void link_rules(char* rule, FW_INSTANCE* instance) /**Apply rules to users*/ - bool match_any; + bool match_any = true; char *tok, *ruleptr, *userptr, *modeptr; char *saveptr = NULL; RULELIST* rulelist = NULL; From 4e5c4c0b6af34a898f5096000e6d32c517046c5a Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Mon, 16 Feb 2015 09:25:41 +0200 Subject: [PATCH 18/18] Fix to bug 568: http://bugs.mariadb.com/show_bug.cgi?id=568 Changed strtok calls to strtok_r where needed. --- rabbitmq_consumer/consumer.c | 5 +++-- server/modules/filter/test/harness_common.c | 14 ++++++++------ server/modules/filter/test/harness_ui.c | 1 - 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/rabbitmq_consumer/consumer.c b/rabbitmq_consumer/consumer.c index 6712fbfe5..cf338f291 100644 --- a/rabbitmq_consumer/consumer.c +++ b/rabbitmq_consumer/consumer.c @@ -158,6 +158,7 @@ int sendMessage(MYSQL* server, amqp_message_t* msg) (int)((msg->properties.correlation_id.len + 1)*2+1) + strlen(DB_INSERT), rval = 0; +char* saved; char *qstr = calloc(buffsz,sizeof(char)), *rawmsg = calloc((msg->body.len + 1),sizeof(char)), *clnmsg = calloc(((msg->body.len + 1)*2+1),sizeof(char)), @@ -170,9 +171,9 @@ int sendMessage(MYSQL* server, amqp_message_t* msg) sprintf(qstr,"%.*s",(int)msg->body.len,(char *)msg->body.bytes); fprintf(out_fd,"Received: %s\n",qstr); - char *ptr = strtok(qstr,"|"); + char *ptr = strtok_r(qstr,"|",&saved); sprintf(rawdate,"%s",ptr); - ptr = strtok(NULL,"\n\0"); + ptr = strtok_r(NULL,"\n\0",&saved); if(ptr == NULL){ fprintf(out_fd,"Message content not valid.\n"); rval = 1; diff --git a/server/modules/filter/test/harness_common.c b/server/modules/filter/test/harness_common.c index 254add4b1..2cfe49b42 100644 --- a/server/modules/filter/test/harness_common.c +++ b/server/modules/filter/test/harness_common.c @@ -141,6 +141,7 @@ FILTER_PARAMETER** read_params(int* paramc) { char buffer[256]; char* token; + char* saveptr; char* names[64]; char* values[64]; int pc = 0, do_read = 1, val_len = 0; @@ -157,14 +158,14 @@ FILTER_PARAMETER** read_params(int* paramc) if(strcmp("done\n",buffer) == 0){ do_read = 0; }else{ - token = strtok(buffer,"=\n"); + token = strtok_r(buffer,"=\n",&saveptr); if(token!=NULL){ val_len = strcspn(token," \n\0"); if((names[pc] = calloc((val_len + 1),sizeof(char))) != NULL){ memcpy(names[pc],token,val_len); } } - token = strtok(NULL,"=\n"); + token = strtok_r(NULL,"=\n",&saveptr); if(token!=NULL){ val_len = strcspn(token," \n\0"); if((values[pc] = calloc((val_len + 1),sizeof(char))) != NULL){ @@ -997,6 +998,7 @@ int process_opts(int argc, char** argv) int fd, buffsize = 1024; int rd,rdsz, rval = 0, error = 0; size_t fsize; + char* saveptr; char *buff = calloc(buffsize,sizeof(char)), *tok = NULL; /**Parse 'harness.cnf' file*/ @@ -1027,16 +1029,16 @@ int process_opts(int argc, char** argv) instance.session_count = 1; rdsz = read(fd,buff,fsize); buff[rdsz] = '\0'; - tok = strtok(buff,"="); + tok = strtok_r(buff,"=",&saveptr); while(tok){ if(!strcmp(tok,"threads")){ - tok = strtok(NULL,"\n\0"); + tok = strtok_r(NULL,"\n\0",&saveptr); instance.thrcount = strtol(tok,0,0); }else if(!strcmp(tok,"sessions")){ - tok = strtok(NULL,"\n\0"); + tok = strtok_r(NULL,"\n\0",&saveptr); instance.session_count = strtol(tok,0,0); } - tok = strtok(NULL,"="); + tok = strtok_r(NULL,"=",&saveptr); } diff --git a/server/modules/filter/test/harness_ui.c b/server/modules/filter/test/harness_ui.c index 8921a88bc..8854b5c49 100644 --- a/server/modules/filter/test/harness_ui.c +++ b/server/modules/filter/test/harness_ui.c @@ -20,7 +20,6 @@ int main(int argc, char** argv){ printf("\n\n\tFilter Test Harness\n\n"); } - while(instance.running){ printf("Harness> "); memset(buffer,0,256);