From ea039dd01aa137f929b5d512d26f880f9867aae4 Mon Sep 17 00:00:00 2001
From: Markus Makela <markus.makela@mariadb.com>
Date: Sat, 7 Mar 2015 06:00:01 +0200
Subject: [PATCH] Fixed to Coverity defects and a fix to tee filter not
 compiling with SS_DEBUG flag.

---
 server/core/resultset.c                       | 13 ++++-----
 server/core/test/testservice.c                |  4 ++-
 server/modules/filter/CMakeLists.txt          |  6 ++---
 server/modules/filter/fwfilter.c              |  2 ++
 .../filter/{lagfilter.c => slavelag.c}        | 27 ++++++++++---------
 server/modules/filter/tee.c                   |  8 +-----
 server/modules/routing/binlog/blr_file.c      |  8 +++---
 server/modules/routing/binlog/blr_master.c    |  4 +--
 server/modules/routing/maxinfo/maxinfo.c      |  4 ++-
 .../modules/routing/maxinfo/maxinfo_error.c   |  5 ++--
 server/modules/routing/maxinfo/maxinfo_exec.c |  2 +-
 .../modules/routing/maxinfo/maxinfo_parse.c   |  6 +++++
 12 files changed, 50 insertions(+), 39 deletions(-)
 rename server/modules/filter/{lagfilter.c => slavelag.c} (92%)

diff --git a/server/core/resultset.c b/server/core/resultset.c
index cca4e3c5a..7f9d88bbf 100644
--- a/server/core/resultset.c
+++ b/server/core/resultset.c
@@ -73,18 +73,19 @@ resultset_free(RESULTSET *resultset)
 {
 RESULT_COLUMN *col;
 
-	if (resultset)
-		return;
-	col = resultset->column;
-	while (col)
+	if (resultset != NULL)
 	{
-	RESULT_COLUMN *next;
+	    col = resultset->column;
+	    while (col)
+	    {
+		RESULT_COLUMN *next;
 
 		next = col->next;
 		resultset_column_free(col);
 		col = next;
+	    }
+	    free(resultset);
 	}
-	free(resultset);
 }
 
 /**
diff --git a/server/core/test/testservice.c b/server/core/test/testservice.c
index 7474925d4..ef2560481 100644
--- a/server/core/test/testservice.c
+++ b/server/core/test/testservice.c
@@ -42,6 +42,7 @@ static bool success = false;
 int hup(DCB* dcb)
 {
     success = true;
+    return 1;
 }
 
 /**
@@ -108,7 +109,8 @@ hkinit();
         skygw_log_sync_all();
         ss_info_dassert(0 != result, "Stop should succeed");
 
-        dcb = dcb_alloc(DCB_ROLE_REQUEST_HANDLER);
+	if((dcb = dcb_alloc(DCB_ROLE_REQUEST_HANDLER)) == NULL)
+	    return 1;
         ss_info_dassert(dcb != NULL, "DCB allocation failed");
         
         session = session_alloc(service,dcb);
diff --git a/server/modules/filter/CMakeLists.txt b/server/modules/filter/CMakeLists.txt
index 98d855d54..84629559b 100644
--- a/server/modules/filter/CMakeLists.txt
+++ b/server/modules/filter/CMakeLists.txt
@@ -34,9 +34,9 @@ add_library(namedserverfilter SHARED namedserverfilter.c)
 target_link_libraries(namedserverfilter log_manager utils)
 install(TARGETS namedserverfilter DESTINATION modules)
 
-add_library(lagfilter SHARED lagfilter.c)
-target_link_libraries(lagfilter log_manager utils query_classifier)
-install(TARGETS lagfilter DESTINATION modules)
+add_library(slavelag SHARED slavelag.c)
+target_link_libraries(slavelag log_manager utils query_classifier)
+install(TARGETS slavelag DESTINATION modules)
 
 
 add_subdirectory(hint)
diff --git a/server/modules/filter/fwfilter.c b/server/modules/filter/fwfilter.c
index d33cacb2e..63d07b5a9 100644
--- a/server/modules/filter/fwfilter.c
+++ b/server/modules/filter/fwfilter.c
@@ -1558,6 +1558,7 @@ bool check_match_any(FW_INSTANCE* my_instance, FW_SESSION* my_session, GWBUF *qu
 		}
 
 		qlen = gw_mysql_get_byte3(memptr);
+		qlen = qlen < 0xffffff ? qlen : 0xffffff;
 		fullquery = malloc((qlen) * sizeof(char));
 		memcpy(fullquery,memptr + 5,qlen - 1);
 		memset(fullquery + qlen - 1,0,1);
@@ -1612,6 +1613,7 @@ bool check_match_all(FW_INSTANCE* my_instance, FW_SESSION* my_session, GWBUF *qu
 		}
 
 		qlen = gw_mysql_get_byte3(memptr);
+		qlen = qlen < 0xffffff ? qlen : 0xffffff;
 		fullquery = malloc((qlen) * sizeof(char));
 		memcpy(fullquery,memptr + 5,qlen - 1);
 		memset(fullquery + qlen - 1,0,1);
diff --git a/server/modules/filter/lagfilter.c b/server/modules/filter/slavelag.c
similarity index 92%
rename from server/modules/filter/lagfilter.c
rename to server/modules/filter/slavelag.c
index aff236850..c12d7a2a7 100644
--- a/server/modules/filter/lagfilter.c
+++ b/server/modules/filter/slavelag.c
@@ -32,7 +32,7 @@ extern size_t         log_ses_count[];
 extern __thread log_info_t tls_log_info;
 
 /**
- * @file lagfilter.c - a very simple filter designed to send queries to the 
+ * @file slavelag.c - a very simple filter designed to send queries to the 
  * master server after data modification has occurred. This is done to prevent
  * replication lag affecting the outcome of a select query.
  * 
@@ -42,8 +42,11 @@ extern __thread log_info_t tls_log_info;
  * is executed:
  *
  *	count=<number of queries>   Queries to route to master after data modification.
- *	time=<time period>	    Seconds to wait before queries are routed to slaves.
+ *	time=<time period>	    	Seconds to wait before queries are routed to slaves.
+ *	match=<regex>				Regex for matching
+ *  ignore=<regex>				Regex for ignoring
  *
+ * The filter also has two options: @c case, which makes the regex case-sensitive, and @c ignorecase, which does the opposite.
  * Date		Who		Description
  * 03/03/2015	Markus Mäkelä	Written for demonstrative purposes
  * @endverbatim
@@ -333,20 +336,20 @@ time_t now = time(NULL);
 
 	    if(query_classifier_get_operation(queue) & (QUERY_OP_DELETE|QUERY_OP_INSERT|QUERY_OP_UPDATE))
 	    {
-		sql = modutil_get_SQL(queue);
-		
-		if(my_instance->nomatch == NULL||(my_instance->nomatch && regexec(&my_instance->nore,sql,0,NULL,0) != 0))
+		if((sql = modutil_get_SQL(queue)) != NULL)
 		{
-		    if(my_instance->match == NULL||
-		     (my_instance->match && regexec(&my_instance->re,sql,0,NULL,0) == 0))
+		    if(my_instance->nomatch == NULL||(my_instance->nomatch && regexec(&my_instance->nore,sql,0,NULL,0) != 0))
 		    {
-			my_session->hints_left = my_instance->count;
-			my_session->last_modification = now;
-			my_instance->stats.n_modified++;
+			if(my_instance->match == NULL||
+			 (my_instance->match && regexec(&my_instance->re,sql,0,NULL,0) == 0))
+			{
+			    my_session->hints_left = my_instance->count;
+			    my_session->last_modification = now;
+			    my_instance->stats.n_modified++;
+			}
 		    }
+		    free(sql);
 		}
-
-		free(sql);
 	    }
 	    else if(my_session->hints_left > 0)
 	    {
diff --git a/server/modules/filter/tee.c b/server/modules/filter/tee.c
index 236549c43..02eca9943 100644
--- a/server/modules/filter/tee.c
+++ b/server/modules/filter/tee.c
@@ -1170,13 +1170,7 @@ clientReply (FILTER* instance, void *session, GWBUF *reply)
 	  if(my_session->waiting[PARENT])
 	    {
 	      route = true;
-#ifdef SS_DEBUG
-	      ss_dassert(my_session->replies[PARENT] < 2 || 
-			 modutil_count_signal_packets(my_session->tee_replybuf,
-						      my_session->use_ok,
-						      my_session->eof[PARENT]) == 0);
-	      skygw_log_write_flush(LOGFILE_DEBUG,"tee.c:[%d] Routing partial response set.",my_session->d_id);
-#endif
+
 	    }
 	  else if(my_session->eof[PARENT] >= min_eof &&
 		  my_session->eof[CHILD] >= min_eof)
diff --git a/server/modules/routing/binlog/blr_file.c b/server/modules/routing/binlog/blr_file.c
index a3b3b7054..b48ed6b41 100644
--- a/server/modules/routing/binlog/blr_file.c
+++ b/server/modules/routing/binlog/blr_file.c
@@ -82,7 +82,7 @@ struct dirent	*dp;
 		strcpy(path, "/usr/local/skysql/MaxScale");
 		if ((ptr = getenv("MAXSCALE_HOME")) != NULL)
 		{
-			strcpy(path, ptr);
+			strncpy(path, ptr,PATH_MAX);
 		}
 		strcat(path, "/");
 		strcat(path, router->service->name);
@@ -196,7 +196,7 @@ unsigned char	magic[] = BINLOG_MAGIC;
 	fsync(fd);
 	close(router->binlog_fd);
 	spinlock_acquire(&router->binlog_lock);
-	strcpy(router->binlog_name, file);
+	strncpy(router->binlog_name, file,BINLOG_FNAMELEN+1);
 	router->binlog_position = 4;			/* Initial position after the magic number */
 	spinlock_release(&router->binlog_lock);
 	router->binlog_fd = fd;
@@ -230,7 +230,7 @@ int		fd;
 	fsync(fd);
 	close(router->binlog_fd);
 	spinlock_acquire(&router->binlog_lock);
-	strcpy(router->binlog_name, file);
+	strncpy(router->binlog_name, file,BINLOG_FNAMELEN+1);
 	router->binlog_position = lseek(fd, 0L, SEEK_END);
 	spinlock_release(&router->binlog_lock);
 	router->binlog_fd = fd;
@@ -310,7 +310,7 @@ BLFILE		*file;
 		spinlock_release(&router->fileslock);
 		return NULL;
 	}
-	strcpy(file->binlogname, binlog);
+	strncpy(file->binlogname, binlog,BINLOG_FNAMELEN+1);
 	file->refcnt = 1;
 	file->cache = 0;
 	spinlock_init(&file->lock);
diff --git a/server/modules/routing/binlog/blr_master.c b/server/modules/routing/binlog/blr_master.c
index 83c95ddf9..4c6ff3cd6 100644
--- a/server/modules/routing/binlog/blr_master.c
+++ b/server/modules/routing/binlog/blr_master.c
@@ -1228,8 +1228,8 @@ MYSQL_session	*auth_info;
 
 	if ((auth_info = calloc(1, sizeof(MYSQL_session))) == NULL)
 		return NULL;
-	strcpy(auth_info->user, username);
-	strcpy(auth_info->db, database);
+	strncpy(auth_info->user, username,MYSQL_USER_MAXLEN+1);
+	strncpy(auth_info->db, database,MYSQL_DATABASE_MAXLEN+1);
 	gw_sha1_str((const uint8_t *)password, strlen(password), auth_info->client_sha1);
 
 	return auth_info;
diff --git a/server/modules/routing/maxinfo/maxinfo.c b/server/modules/routing/maxinfo/maxinfo.c
index ac635360c..1aa679b7f 100644
--- a/server/modules/routing/maxinfo/maxinfo.c
+++ b/server/modules/routing/maxinfo/maxinfo.c
@@ -385,7 +385,9 @@ char		*sql;
 	if (modutil_MySQL_Query(queue, &sql, &len, &residual))
 	{
 		sql = strndup(sql, len);
-		return maxinfo_execute_query(instance, session, sql);
+		int rc = maxinfo_execute_query(instance, session, sql);
+		free(sql);
+		return rc;
 	}
 	else
 	{
diff --git a/server/modules/routing/maxinfo/maxinfo_error.c b/server/modules/routing/maxinfo/maxinfo_error.c
index 7ec88e73d..dd5a8f233 100644
--- a/server/modules/routing/maxinfo/maxinfo_error.c
+++ b/server/modules/routing/maxinfo/maxinfo_error.c
@@ -85,6 +85,7 @@ int	len;
 		return;
 	sprintf(msg, "%s in query '%s'", desc, sql);
 	maxinfo_send_error(dcb, 1149, msg);
+	free(msg);
 }
 
 /**
@@ -112,7 +113,7 @@ int             len;
         data[4] = 0xff;				// Error indicator
 	data[5] = errcode & 0xff;		// Error Code
 	data[6] = (errcode >> 8) & 0xff;	// Error Code
-	strncpy((char *)&data[7], "#42000", 6);
-        strncpy((char *)&data[13], msg, strlen(msg));	// Error Message
+	memcpy(&data[7], "#42000", 6);
+        memcpy(&data[13], msg, strlen(msg));	// Error Message
 	dcb->func.write(dcb, pkt);
 }
diff --git a/server/modules/routing/maxinfo/maxinfo_exec.c b/server/modules/routing/maxinfo/maxinfo_exec.c
index 404367d50..cced4bec5 100644
--- a/server/modules/routing/maxinfo/maxinfo_exec.c
+++ b/server/modules/routing/maxinfo/maxinfo_exec.c
@@ -735,7 +735,7 @@ exec_select(DCB *dcb, MAXINFO_TREE *tree)
 static int
 maxinfo_pattern_match(char *pattern, char *str)
 {
-int	anchor, len, trailing;
+int	anchor = 0, len, trailing;
 char	*fixed;
 extern	char *strcasestr();
 
diff --git a/server/modules/routing/maxinfo/maxinfo_parse.c b/server/modules/routing/maxinfo/maxinfo_parse.c
index 14dfad594..06e27dd75 100644
--- a/server/modules/routing/maxinfo/maxinfo_parse.c
+++ b/server/modules/routing/maxinfo/maxinfo_parse.c
@@ -100,6 +100,7 @@ MAXINFO_TREE	*col, *table;
 				}
 			}
 			// Malformed show
+			free(text);
 			free_tree(tree);
 			*parse_error = PARSE_MALFORMED_SHOW;
 			return NULL;
@@ -151,6 +152,8 @@ MAXINFO_TREE	*tree = NULL;
 		case LT_FROM:
 			return make_tree_node(MAXOP_COLUMNS, text, NULL, 
 				NULL);
+		default:
+		    free(text);
 		}
 		break;
 	case LT_STAR:
@@ -159,6 +162,8 @@ MAXINFO_TREE	*tree = NULL;
 		if (lookahead != LT_FROM)
 			return make_tree_node(MAXOP_ALL_COLUMNS, NULL, NULL,
 				NULL);
+	default:
+	    free(text2);
 	}
 	return NULL;
 }
@@ -180,6 +185,7 @@ MAXINFO_TREE	*tree = NULL;
 	*ptr = fetch_token(*ptr, &token, &text);
 	if  (token == LT_STRING)
 		return make_tree_node(MAXOP_TABLE, text, NULL, NULL);
+	free(text);
 	return NULL;
 }