hcq/MaxScale
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fixes to Coverity issues 84388, 84386, 84385

Browse Source 
mysql_mon.c: Added back server state logging to Debug build.
query_classifier.cc: removed some extra debug code, cleaned up a bit function documentation.
mysql_client_server_protocol.h, mysql_backend.c, mysql_common.c: changed some variables to signed ones to enable checking of calculations in the code.
skygw_utils.cc: removed erroneous debug assertion.
This commit is contained in:
VilhoRaatikka
2014-12-05 23:39:14 +02:00
parent 6d98df0c37
commit 01b1b0a304
6 changed files with 58 additions and 65 deletions
Download Patch File Download Diff File Expand all files Collapse all files

76
query_classifier/query_classifier.cc
View File

@ -362,50 +362,29 @@ static bool create_parse_tree(
Parser_state parser_state; Parser_state parser_state;
bool failp = FALSE; bool failp = FALSE;
const char* virtual_db = "skygw_virtual"; const char* virtual_db = "skygw_virtual";
#if defined(SS_DEBUG_EXTRA)
LOGIF(LM, (skygw_log_write_flush( if (parser_state.init(thd, thd->query(), thd->query_length()))
LOGFILE_MESSAGE, {
"[readwritesplit:create_parse_tree] 1.")));
#endif
if (parser_state.init(thd, thd->query(), thd->query_length())) {
failp = TRUE; failp = TRUE;
goto return_here; goto return_here;
} }
#if defined(SS_DEBUG_EXTRA)
LOGIF(LM, (skygw_log_write_flush(
LOGFILE_MESSAGE,
"[readwritesplit:create_parse_tree] 2.")));
#endif
mysql_reset_thd_for_next_command(thd); mysql_reset_thd_for_next_command(thd);
#if defined(SS_DEBUG_EXTRA)
LOGIF(LM, (skygw_log_write_flush(
LOGFILE_MESSAGE,
"[readwritesplit:create_parse_tree] 3.")));
#endif
/** /**
* Set some database to thd so that parsing won't fail because of * Set some database to thd so that parsing won't fail because of
* missing database. Then parse. * missing database. Then parse.
*/ */
failp = thd->set_db(virtual_db, strlen(virtual_db)); failp = thd->set_db(virtual_db, strlen(virtual_db));
#if defined(SS_DEBUG_EXTRA) if (failp)
LOGIF(LM, (skygw_log_write_flush( {
LOGFILE_MESSAGE,
"[readwritesplit:create_parse_tree] 4.")));
#endif
if (failp) {
LOGIF(LE, (skygw_log_write_flush( LOGIF(LE, (skygw_log_write_flush(
LOGFILE_ERROR, LOGFILE_ERROR,
"Error : Failed to set database in thread context."))); "Error : Failed to set database in thread context.")));
} }
failp = parse_sql(thd, &parser_state, NULL); failp = parse_sql(thd, &parser_state, NULL);
#if defined(SS_DEBUG_EXTRA) if (failp)
LOGIF(LM, (skygw_log_write_flush( {
LOGFILE_MESSAGE,
"[readwritesplit:create_parse_tree] 5.")));
#endif
if (failp) {
LOGIF(LD, (skygw_log_write( LOGIF(LD, (skygw_log_write(
LOGFILE_DEBUG, LOGFILE_DEBUG,
"%lu [readwritesplit:create_parse_tree] failed to " "%lu [readwritesplit:create_parse_tree] failed to "
@ -417,16 +396,14 @@ return_here:
} }
/** /**
* @node Set new query type if new is more restrictive than old. * Set new query type if new is more restrictive than old.
* *
* Parameters: * Parameters:
* @param qtype - <usage> * @param qtype Existing type
* <description>
* *
* @param new_type - <usage> * @param new_type New query type
* <description>
* *
* @return * @return Query type as an unsigned int value which must be casted to qtype.
* *
* *
* @details The implementation relies on that enumerated values correspond * @details The implementation relies on that enumerated values correspond
@ -443,13 +420,11 @@ static u_int32_t set_query_type(
} }
/** /**
* @node Detect query type, read-only, write, or session update * Detect query type by examining parsed representation of it.
* *
* Parameters: * @param thd MariaDB thread context.
* @param thd - <usage>
* <description>
* *
* @return * @return Copy of query type value.
* *
* *
* @details Query type is deduced by checking for certain properties * @details Query type is deduced by checking for certain properties
@ -474,7 +449,7 @@ static skygw_query_type_t resolve_query_type(
* all write operations to all nodes. * all write operations to all nodes.
*/ */
#if defined(NOT_IN_USE) #if defined(NOT_IN_USE)
bool force_data_modify_op_replication; bool force_data_modify_op_replication;
force_data_modify_op_replication = FALSE; force_data_modify_op_replication = FALSE;
#endif /* NOT_IN_USE */ #endif /* NOT_IN_USE */
ss_info_dassert(thd != NULL, ("thd is NULL\n")); ss_info_dassert(thd != NULL, ("thd is NULL\n"));
@ -868,6 +843,11 @@ return_qtype:
* Checks if statement causes implicit COMMIT. * Checks if statement causes implicit COMMIT.
* autocommit_stmt gets values 1, 0 or -1 if stmt is enable, disable or * autocommit_stmt gets values 1, 0 or -1 if stmt is enable, disable or
* something else than autocommit. * something else than autocommit.
*
* @param lex Parse tree
* @param autocommit_stmt memory address for autocommit status
*
* @return true if statement causes implicit commit and false otherwise
*/ */
static bool skygw_stmt_causes_implicit_commit( static bool skygw_stmt_causes_implicit_commit(
LEX* lex, LEX* lex,
@ -897,7 +877,7 @@ static bool skygw_stmt_causes_implicit_commit(
} }
else else
{ {
succp =false; succp = false;
} }
break; break;
default: default:
@ -913,7 +893,9 @@ return_succp:
* Finds out if stmt is SET autocommit * Finds out if stmt is SET autocommit
* and if the new value matches with the enable_cmd argument. * and if the new value matches with the enable_cmd argument.
* *
* Returns 1, 0, or -1 if command was: * @param lex parse tree
*
* @return 1, 0, or -1 if command was:
* enable, disable, or not autocommit, respectively. * enable, disable, or not autocommit, respectively.
*/ */
static int is_autocommit_stmt( static int is_autocommit_stmt(
@ -984,9 +966,11 @@ char* skygw_query_classifier_get_stmtname(
} }
/** /**
*Returns the LEX struct of the parsed GWBUF * Get the parse tree from parsed querybuf.
*@param The parsed GWBUF * @param querybuf The parsed GWBUF
*@return Pointer to the LEX struct or NULL if an error occurred or the query was not parsed *
* @return Pointer to the LEX struct or NULL if an error occurred or the query
* was not parsed
*/ */
LEX* get_lex(GWBUF* querybuf) LEX* get_lex(GWBUF* querybuf)
{ {
@ -1195,7 +1179,7 @@ bool is_drop_table_query(GWBUF* querybuf)
lex->sql_command == SQLCOM_DROP_TABLE; lex->sql_command == SQLCOM_DROP_TABLE;
} }
/* /**
* Replace user-provided literals with question marks. Return a copy of the * Replace user-provided literals with question marks. Return a copy of the
* querystr with replacements. * querystr with replacements.
* *

8
server/modules/include/mysql_client_server_protocol.h
View File

@ -252,7 +252,7 @@ typedef enum mysql_server_cmd {
typedef struct server_command_st { typedef struct server_command_st {
mysql_server_cmd_t scom_cmd; mysql_server_cmd_t scom_cmd;
int scom_nresponse_packets; /*< packets in response */ int scom_nresponse_packets; /*< packets in response */
size_t scom_nbytes_to_read; /*< bytes left to read in current packet */ ssize_t scom_nbytes_to_read; /*< bytes left to read in current packet */
struct server_command_st* scom_next; struct server_command_st* scom_next;
} server_command_t; } server_command_t;
@ -388,8 +388,8 @@ void protocol_remove_srv_command(MySQLProtocol* p);
bool protocol_waits_response(MySQLProtocol* p); bool protocol_waits_response(MySQLProtocol* p);
mysql_server_cmd_t protocol_get_srv_command(MySQLProtocol* p,bool removep); mysql_server_cmd_t protocol_get_srv_command(MySQLProtocol* p,bool removep);
int get_stmt_nresponse_packets(GWBUF* buf, mysql_server_cmd_t cmd); int get_stmt_nresponse_packets(GWBUF* buf, mysql_server_cmd_t cmd);
bool protocol_get_response_status (MySQLProtocol* p, int* npackets, size_t* nbytes); bool protocol_get_response_status (MySQLProtocol* p, int* npackets, ssize_t* nbytes);
void protocol_set_response_status (MySQLProtocol* p, int npackets, size_t nbytes); void protocol_set_response_status (MySQLProtocol* p, int npackets, ssize_t nbytes);
void protocol_archive_srv_command(MySQLProtocol* p); void protocol_archive_srv_command(MySQLProtocol* p);
@ -397,6 +397,6 @@ void init_response_status (
GWBUF* buf, GWBUF* buf,
mysql_server_cmd_t cmd, mysql_server_cmd_t cmd,
int* npackets, int* npackets,
size_t* nbytes); ssize_t* nbytes);

13
server/modules/monitor/mysql_mon.c
View File

@ -676,12 +676,21 @@ int log_no_master = 1;
if (mon_status_changed(ptr)) if (mon_status_changed(ptr))
{ {
LOGIF(LD, (skygw_log_write_flush( #if defined(SS_DEBUG)
LOGFILE_DEBUG, LOGIF(LT, (skygw_log_write_flush(
LOGFILE_TRACE,
"Backend server %s:%d state : %s", "Backend server %s:%d state : %s",
ptr->server->name, ptr->server->name,
ptr->server->port, ptr->server->port,
STRSRVSTATUS(ptr->server)))); STRSRVSTATUS(ptr->server))));
#else
LOGIF(LD, (skygw_log_write_flush(
LOGFILE_DEBUG,
"Backend server %s:%d state : %s",
ptr->server->name,
ptr->server->port,
STRSRVSTATUS(ptr->server))));
#endif
} }
if (SERVER_IS_DOWN(ptr->server)) if (SERVER_IS_DOWN(ptr->server))

17
server/modules/protocol/mysql_backend.c
View File

@ -835,7 +835,7 @@ static int gw_error_backend_event(DCB *dcb)
len = sizeof(error); len = sizeof(error);
if (getsockopt(dcb->fd, SOL_SOCKET, SO_ERROR, &error, &len) == 0) if (getsockopt(dcb->fd, SOL_SOCKET, SO_ERROR, &error, (socklen_t *)&len) == 0)
{ {
if (error != 0) if (error != 0)
{ {
@ -877,7 +877,7 @@ static int gw_error_backend_event(DCB *dcb)
char buf[100]; char buf[100];
len = sizeof(error); len = sizeof(error);
if (getsockopt(dcb->fd, SOL_SOCKET, SO_ERROR, &error, &len) == 0) if (getsockopt(dcb->fd, SOL_SOCKET, SO_ERROR, &error, (socklen_t *)&len) == 0)
{ {
if (error != 0) if (error != 0)
{ {
@ -1083,7 +1083,7 @@ gw_backend_hangup(DCB *dcb)
char buf[100]; char buf[100];
len = sizeof(error); len = sizeof(error);
if (getsockopt(dcb->fd, SOL_SOCKET, SO_ERROR, &error, &len) == 0) if (getsockopt(dcb->fd, SOL_SOCKET, SO_ERROR, &error, (socklen_t *)&len) == 0)
{ {
if (error != 0) if (error != 0)
{ {
@ -1334,7 +1334,6 @@ static int gw_change_user(
/* get the auth token len */ /* get the auth token len */
memcpy(&auth_token_len, client_auth_packet, 1); memcpy(&auth_token_len, client_auth_packet, 1);
ss_dassert(auth_token_len >= 0);
client_auth_packet++; client_auth_packet++;
@ -1483,7 +1482,7 @@ static GWBUF* process_response_data (
int nbytes_to_process) /*< number of new bytes read */ int nbytes_to_process) /*< number of new bytes read */
{ {
int npackets_left = 0; /*< response's packet count */ int npackets_left = 0; /*< response's packet count */
size_t nbytes_left = 0; /*< nbytes to be read for the packet */ ssize_t nbytes_left = 0; /*< nbytes to be read for the packet */
MySQLProtocol* p; MySQLProtocol* p;
GWBUF* outbuf = NULL; GWBUF* outbuf = NULL;
@ -1557,11 +1556,13 @@ static GWBUF* process_response_data (
*/ */
else /*< nbytes_left < nbytes_to_process */ else /*< nbytes_left < nbytes_to_process */
{ {
ss_dassert(nbytes_left >= 0);
nbytes_to_process -= nbytes_left; nbytes_to_process -= nbytes_left;
/** Move the prefix of the buffer to outbuf from redbuf */ /** Move the prefix of the buffer to outbuf from redbuf */
outbuf = gwbuf_append(outbuf, gwbuf_clone_portion(readbuf, 0, nbytes_left)); outbuf = gwbuf_append(outbuf,
readbuf = gwbuf_consume(readbuf, nbytes_left); gwbuf_clone_portion(readbuf, 0, (size_t)nbytes_left));
readbuf = gwbuf_consume(readbuf, (size_t)nbytes_left);
ss_dassert(npackets_left > 0); ss_dassert(npackets_left > 0);
npackets_left -= 1; npackets_left -= 1;
nbytes_left = 0; nbytes_left = 0;
@ -1609,7 +1610,7 @@ static bool sescmd_response_complete(
DCB* dcb) DCB* dcb)
{ {
int npackets_left; int npackets_left;
size_t nbytes_left; ssize_t nbytes_left;
MySQLProtocol* p; MySQLProtocol* p;
bool succp; bool succp;

8
server/modules/protocol/mysql_common.c
View File

@ -1965,7 +1965,7 @@ void init_response_status (
GWBUF* buf, GWBUF* buf,
mysql_server_cmd_t cmd, mysql_server_cmd_t cmd,
int* npackets, int* npackets,
size_t* nbytes_left) ssize_t* nbytes_left)
{ {
uint8_t* packet; uint8_t* packet;
int nparam; int nparam;
@ -2027,7 +2027,7 @@ void init_response_status (
bool protocol_get_response_status ( bool protocol_get_response_status (
MySQLProtocol* p, MySQLProtocol* p,
int* npackets, int* npackets,
size_t* nbytes) ssize_t* nbytes)
{ {
bool succp; bool succp;
@ -2035,7 +2035,7 @@ bool protocol_get_response_status (
spinlock_acquire(&p->protocol_lock); spinlock_acquire(&p->protocol_lock);
*npackets = p->protocol_command.scom_nresponse_packets; *npackets = p->protocol_command.scom_nresponse_packets;
*nbytes = p->protocol_command.scom_nbytes_to_read; *nbytes = (ssize_t)p->protocol_command.scom_nbytes_to_read;
spinlock_release(&p->protocol_lock); spinlock_release(&p->protocol_lock);
if (*npackets < 0 && *nbytes == 0) if (*npackets < 0 && *nbytes == 0)
@ -2053,7 +2053,7 @@ bool protocol_get_response_status (
void protocol_set_response_status ( void protocol_set_response_status (
MySQLProtocol* p, MySQLProtocol* p,
int npackets_left, int npackets_left,
size_t nbytes) ssize_t nbytes)
{ {
CHK_PROTOCOL(p); CHK_PROTOCOL(p);

1
utils/skygw_utils.cc
View File

@ -1412,7 +1412,6 @@ int simple_mutex_unlock(
err, err,
strerror(errno)); strerror(errno));
perror("simple_mutex : "); perror("simple_mutex : ");
ss_dassert(sm->sm_mutex.__data.__nusers >= 0);
} else { } else {
/** /**
* Note that these updates are not protected. * Note that these updates are not protected.