From 061fa62d29a9efa10799824525a0127f1cda7a43 Mon Sep 17 00:00:00 2001 From: VilhoRaatikka Date: Tue, 11 Nov 2014 14:10:06 +0200 Subject: [PATCH] Fixes to Coverity issues 77197 (mysql_common.c, dcb.c, mysql_client.c, skygw_debug.h), 72654 (poll.c), 72756 (mysql_backend.c), 72744 (mysql_backend.c), 77197 (mysql_common.c), 72746 (mysql_common.c), 72676 (mysql_common.c), 72705 (readwritesplit.c), 72697 (readwritesplit.c), 72652 (skygw_debug.h) --- server/core/dcb.c | 6 +- server/core/poll.c | 15 +- server/modules/protocol/mysql_backend.c | 70 +++++----- server/modules/protocol/mysql_client.c | 8 +- server/modules/protocol/mysql_common.c | 128 +++++++++--------- .../routing/readwritesplit/readwritesplit.c | 18 ++- utils/skygw_debug.h | 7 +- 7 files changed, 138 insertions(+), 114 deletions(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index da0f123e8..4e15aa90e 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -509,10 +509,14 @@ bool succp = false; pthread_self(), dcb->fd, dcb))); +#endif /* SS_DEBUG */ +#if defined(FAKE_CODE) conn_open[dcb->fd] = false; +#endif /* FAKE_CODE */ +#if defined(SS_DEBUG) ss_debug(dcb->fd = -1;) } -#endif +#endif /* SS_DEBUG */ succp = dcb_set_state(dcb, DCB_STATE_DISCONNECTED, NULL); ss_dassert(succp); dcb_next = dcb->memdata.next; diff --git a/server/core/poll.c b/server/core/poll.c index 6bc730189..36e118b72 100644 --- a/server/core/poll.c +++ b/server/core/poll.c @@ -1073,13 +1073,26 @@ double avg1 = 0.0, avg5 = 0.0, avg15 = 0.0; { char *event_string = event_to_string(thread_data[i].event); + bool from_heap; + if (event_string == NULL) + { + from_heap = false; event_string = "??"; + } + else + { + from_heap = true; + } dcb_printf(dcb, " %2d | %-10s | %6d | %-16p | %s\n", i, state, thread_data[i].n_fds, thread_data[i].cur_dcb, event_string); - free(event_string); + + if (from_heap) + { + free(event_string); + } } } } diff --git a/server/modules/protocol/mysql_backend.c b/server/modules/protocol/mysql_backend.c index 2ec3712c5..ac36f34d8 100644 --- a/server/modules/protocol/mysql_backend.c +++ b/server/modules/protocol/mysql_backend.c @@ -270,6 +270,11 @@ static int gw_read_backend_event(DCB *dcb) { CHK_SESSION(session); + if (session == NULL) + { + rc = 0; + goto return_with_lock; + } router = session->service->router; router_instance = session->service->router_instance; rsession = session->router_session; @@ -1164,43 +1169,42 @@ static int backend_write_delayqueue(DCB *dcb) SESSION *session = dcb->session; CHK_SESSION(session); - - router = session->service->router; - router_instance = session->service->router_instance; - rsession = session->router_session; + + if (session != NULL) + { + router = session->service->router; + router_instance = session->service->router_instance; + rsession = session->router_session; #if defined(SS_DEBUG) - LOGIF(LE, (skygw_log_write_flush( - LOGFILE_ERROR, - "Backend write delayqueue error handling."))); + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Backend write delayqueue error handling."))); #endif - errbuf = mysql_create_custom_error( - 1, - 0, - "Failed to write buffered data to back-end server. " - "Buffer was empty or back-end was disconnected during " - "operation. Attempting to find a new backend."); - - router->handleError(router_instance, - rsession, - errbuf, - dcb, - ERRACT_NEW_CONNECTION, - &succp); - gwbuf_free(errbuf); + errbuf = mysql_create_custom_error( + 1, + 0, + "Failed to write buffered data to back-end server. " + "Buffer was empty or back-end was disconnected during " + "operation. Attempting to find a new backend."); + + router->handleError(router_instance, + rsession, + errbuf, + dcb, + ERRACT_NEW_CONNECTION, + &succp); + gwbuf_free(errbuf); - if (!succp) - { - if (session != NULL) - { + if (!succp) + { spinlock_acquire(&session->ses_lock); session->state = SESSION_STATE_STOPPING; spinlock_release(&session->ses_lock); - } - ss_dassert(dcb->dcb_errhandle_called); - dcb_close(dcb); - } - } - + ss_dassert(dcb->dcb_errhandle_called); + dcb_close(dcb); + } + } + } return rc; } @@ -1369,8 +1373,8 @@ static GWBUF* process_response_data ( GWBUF* readbuf, int nbytes_to_process) /*< number of new bytes read */ { - int npackets_left = 0; /*< response's packet count */ - size_t nbytes_left = 0; /*< nbytes to be read for the packet */ + int npackets_left = 0; /*< response's packet count */ + size_t nbytes_left = 0; /*< nbytes to be read for the packet */ MySQLProtocol* p; GWBUF* outbuf = NULL; diff --git a/server/modules/protocol/mysql_client.c b/server/modules/protocol/mysql_client.c index 15ecf6852..e8824b957 100644 --- a/server/modules/protocol/mysql_client.c +++ b/server/modules/protocol/mysql_client.c @@ -1064,9 +1064,9 @@ int gw_MySQLListener( strerror(errno)); return 0; } -#if defined(SS_DEBUG) +#if defined(FAKE_CODE) conn_open[l_so] = true; -#endif +#endif /* FAKE_CODE */ listen_dcb->func.accept = gw_MySQLAccept; return 1; @@ -1197,8 +1197,10 @@ int gw_MySQLAccept(DCB *listener) "%lu [gw_MySQLAccept] Accepted fd %d.", pthread_self(), c_sock))); +#endif /* SS_DEBUG */ +#if defined(FAKE_CODE) conn_open[c_sock] = true; -#endif +#endif /* FAKE_CODE */ /* set nonblocking */ sendbuf = GW_CLIENT_SO_SNDBUF; setsockopt(c_sock, SOL_SOCKET, SO_SNDBUF, &sendbuf, optlen); diff --git a/server/modules/protocol/mysql_common.c b/server/modules/protocol/mysql_common.c index e54ea24ee..bfa45ed98 100644 --- a/server/modules/protocol/mysql_common.c +++ b/server/modules/protocol/mysql_common.c @@ -736,13 +736,13 @@ int gw_send_authentication_to_backend( * */ int gw_do_connect_to_backend( - char *host, - int port, - int* fd) + char *host, + int port, + int *fd) { struct sockaddr_in serv_addr; - int rv; - int so = 0; + int rv; + int so = 0; int bufsize; memset(&serv_addr, 0, sizeof serv_addr); @@ -750,8 +750,6 @@ int gw_do_connect_to_backend( so = socket(AF_INET,SOCK_STREAM,0); if (so < 0) { - int eno = errno; - errno = 0; LOGIF(LE, (skygw_log_write_flush( LOGFILE_ERROR, "Error: Establishing connection to backend server " @@ -759,8 +757,8 @@ int gw_do_connect_to_backend( "due %d, %s.", host, port, - eno, - strerror(eno)))); + errno, + strerror(errno)))); rv = -1; goto return_rv; } @@ -769,29 +767,25 @@ int gw_do_connect_to_backend( serv_addr.sin_port = htons(port); bufsize = GW_BACKEND_SO_SNDBUF; - 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; - } - + if(setsockopt(so, SOL_SOCKET, SO_SNDBUF, &bufsize, sizeof(bufsize)) != 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, + errno, + strerror(errno)))); + rv = -1; + /** Close socket */ + goto close_so; + } bufsize = GW_BACKEND_SO_RCVBUF; - if(setsockopt(so, SOL_SOCKET, SO_RCVBUF, &bufsize, sizeof(bufsize)) != 0) - { - int eno = errno; - errno = 0; + if(setsockopt(so, SOL_SOCKET, SO_RCVBUF, &bufsize, sizeof(bufsize)) != 0) + { LOGIF(LE, (skygw_log_write_flush( LOGFILE_ERROR, "Error: Failed to set socket options " @@ -799,49 +793,35 @@ int gw_do_connect_to_backend( "due %d, %s.", host, port, - eno, - strerror(eno)))); - rv = -1; - goto return_rv; - } + errno, + strerror(errno)))); + rv = -1; + /** Close socket */ + goto close_so; + } /* set socket to as non-blocking here */ setnonblocking(so); rv = connect(so, (struct sockaddr *)&serv_addr, sizeof(serv_addr)); - if (rv != 0) { - int eno = errno; - errno = 0; - - if (eno == EINPROGRESS) { + if (rv != 0) + { + if (errno == EINPROGRESS) + { rv = 1; - } else { - int rc; - int oldfd = so; - + } + else + { LOGIF(LE, (skygw_log_write_flush( LOGFILE_ERROR, "Error: Failed to connect backend server %s:%d, " "due %d, %s.", host, port, - eno, - strerror(eno)))); - /*< Close newly created socket. */ - rc = close(so); - - if (rc != 0) { - int eno = errno; - errno = 0; - LOGIF(LE, (skygw_log_write_flush( - LOGFILE_ERROR, - "Error: Failed to " - "close socket %d due %d, %s.", - oldfd, - eno, - strerror(eno)))); - } - goto return_rv; + errno, + strerror(errno)))); + /** Close socket */ + goto close_so; } } *fd = so; @@ -853,11 +833,26 @@ int gw_do_connect_to_backend( host, port, so))); -#if defined(SS_DEBUG) +#if defined(FAKE_CODE) conn_open[so] = true; -#endif +#endif /* FAKE_CODE */ + return_rv: return rv; + +close_so: + /*< Close newly created socket. */ + if (close(so) != 0) + { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Error: Failed to " + "close socket %d due %d, %s.", + so, + errno, + strerror(errno)))); + } + goto return_rv; } /** @@ -1770,11 +1765,14 @@ void protocol_archive_srv_command( { p->protocol_cmd_history = server_command_copy(s1); } - else + else /*< scan and count history commands */ { + len += 1; + while (h1->scom_next != NULL) { h1 = h1->scom_next; + len += 1; } h1->scom_next = server_command_copy(s1); } @@ -2067,7 +2065,7 @@ char* get_username_from_auth( first_letter = (char *)(data + 4 + 4 + 4 + 1 + 23); - if (first_letter == '\0') + if (*first_letter == '\0') { rval = NULL; goto retblock; diff --git a/server/modules/routing/readwritesplit/readwritesplit.c b/server/modules/routing/readwritesplit/readwritesplit.c index fef6c081e..ad30b831a 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.c +++ b/server/modules/routing/readwritesplit/readwritesplit.c @@ -851,7 +851,9 @@ static void* newSession( rses_end_locked_router_action(client_rses); - /** Both Master and at least 1 slave must be found */ + /** + * Master and at least slaves must be found + */ if (!succp) { free(client_rses->rses_backend_ref); free(client_rses); @@ -4101,15 +4103,17 @@ static void handleError ( } session = backend_dcb->session; - if (session != NULL) - CHK_SESSION(session); + if (session == NULL || rses == NULL) + { + *succp = false; + return; + } + CHK_SESSION(session); + CHK_CLIENT_RSES(rses); switch (action) { case ERRACT_NEW_CONNECTION: - { - if (rses != NULL) - CHK_CLIENT_RSES(rses); - + { if (!rses_begin_locked_router_action(rses)) { *succp = false; diff --git a/utils/skygw_debug.h b/utils/skygw_debug.h index 3d43451b5..7294a7ba6 100644 --- a/utils/skygw_debug.h +++ b/utils/skygw_debug.h @@ -183,8 +183,7 @@ typedef enum skygw_chk_t { ((p) == MYSQL_COM_QUIT ? "COM_QUIT" : \ ((p) == MYSQL_COM_STMT_PREPARE ? "MYSQL_COM_STMT_PREPARE" : \ ((p) == MYSQL_COM_STMT_EXECUTE ? "MYSQL_COM_STMT_EXECUTE" : \ - ((p) == MYSQL_COM_UNDEFINED ? "MYSQL_COM_UNDEFINED" : \ - "UNKNOWN MYSQL PACKET TYPE"))))))))))))))))))) + "UNKNOWN MYSQL PACKET TYPE")))))))))))))))))) #define STRDCBSTATE(s) ((s) == DCB_STATE_ALLOC ? "DCB_STATE_ALLOC" : \ ((s) == DCB_STATE_POLLING ? "DCB_STATE_POLLING" : \ @@ -531,8 +530,8 @@ typedef enum skygw_chk_t { -#if defined(SS_DEBUG) +#if defined(FAKE_CODE) bool conn_open[10240]; -#endif +#endif /* FAKE_CODE */ #endif /* SKYGW_DEBUG_H */