Fixes to Coverity tasks : 73267, 72686, 72672

Cleaned up warnings, and added checks to malloc return values and error log writes in case of failures.
This commit is contained in:
VilhoRaatikka
2014-10-31 15:25:59 +02:00
parent 848c7aa0b8
commit 00fded016b
9 changed files with 123 additions and 32 deletions

View File

@ -273,7 +273,6 @@ static char* add_slash(char* str);
static bool check_file_and_path( static bool check_file_and_path(
char* filename, char* filename,
bool* nameconflict,
bool* writable); bool* writable);
static bool file_is_symlink(char* filename); static bool file_is_symlink(char* filename);

View File

@ -15,7 +15,8 @@
* *
* Copyright MariaDB Corporation Ab 2013-2014 * Copyright MariaDB Corporation Ab 2013-2014
*/ */
#if !defined(LOG_MANAGER_H)
# define LOG_MANAGER_H
typedef struct filewriter_st filewriter_t; typedef struct filewriter_st filewriter_t;
typedef struct logfile_st logfile_t; typedef struct logfile_st logfile_t;
@ -90,3 +91,5 @@ const char* get_msg_suffix_default(void);
const char* get_err_prefix_default(void); const char* get_err_prefix_default(void);
const char* get_err_suffix_default(void); const char* get_err_suffix_default(void);
const char* get_logpath_default(void); const char* get_logpath_default(void);
#endif /** LOG_MANAGER_H */

View File

@ -44,6 +44,10 @@
#include <skygw_debug.h> #include <skygw_debug.h>
#include <spinlock.h> #include <spinlock.h>
#include <hint.h> #include <hint.h>
#include <log_manager.h>
#include <errno.h>
extern int lm_enabled_logfiles_bitmask;
static buffer_object_t* gwbuf_remove_buffer_object( static buffer_object_t* gwbuf_remove_buffer_object(
GWBUF* buf, GWBUF* buf,
@ -70,22 +74,23 @@ SHARED_BUF *sbuf;
/* Allocate the buffer header */ /* Allocate the buffer header */
if ((rval = (GWBUF *)malloc(sizeof(GWBUF))) == NULL) if ((rval = (GWBUF *)malloc(sizeof(GWBUF))) == NULL)
{ {
return NULL; goto retblock;;
} }
/* Allocate the shared data buffer */ /* Allocate the shared data buffer */
if ((sbuf = (SHARED_BUF *)malloc(sizeof(SHARED_BUF))) == NULL) if ((sbuf = (SHARED_BUF *)malloc(sizeof(SHARED_BUF))) == NULL)
{ {
free(rval); free(rval);
return NULL; goto retblock;
} }
/* Allocate the space for the actual data */ /* Allocate the space for the actual data */
if ((sbuf->data = (unsigned char *)malloc(size)) == NULL) if ((sbuf->data = (unsigned char *)malloc(size)) == NULL)
{ {
ss_dassert(sbuf->data != NULL);
free(rval); free(rval);
free(sbuf); free(sbuf);
return NULL; goto retblock;
} }
spinlock_init(&rval->gwbuf_lock); spinlock_init(&rval->gwbuf_lock);
rval->start = sbuf->data; rval->start = sbuf->data;
@ -100,6 +105,15 @@ SHARED_BUF *sbuf;
rval->gwbuf_info = GWBUF_INFO_NONE; rval->gwbuf_info = GWBUF_INFO_NONE;
rval->gwbuf_bufobj = NULL; rval->gwbuf_bufobj = NULL;
CHK_GWBUF(rval); CHK_GWBUF(rval);
retblock:
if (rval == NULL || sbuf == NULL || sbuf->data == NULL)
{
ss_dassert(rval != NULL && sbuf != NULL && sbuf->data != NULL);
LOGIF(LE, (skygw_log_write_flush(
LOGFILE_ERROR,
"Error : Memory allocation failed due to %s.",
strerror(errno))));
}
return rval; return rval;
} }
@ -163,6 +177,11 @@ GWBUF *rval;
if ((rval = (GWBUF *)malloc(sizeof(GWBUF))) == NULL) if ((rval = (GWBUF *)malloc(sizeof(GWBUF))) == NULL)
{ {
ss_dassert(rval != NULL);
LOGIF(LE, (skygw_log_write_flush(
LOGFILE_ERROR,
"Error : Memory allocation failed due to %s.",
strerror(errno))));
return NULL; return NULL;
} }
@ -194,6 +213,11 @@ GWBUF *gwbuf_clone_portion(
if ((clonebuf = (GWBUF *)malloc(sizeof(GWBUF))) == NULL) if ((clonebuf = (GWBUF *)malloc(sizeof(GWBUF))) == NULL)
{ {
ss_dassert(clonebuf != NULL);
LOGIF(LE, (skygw_log_write_flush(
LOGFILE_ERROR,
"Error : Memory allocation failed due to %s.",
strerror(errno))));
return NULL; return NULL;
} }
atomic_add(&buf->sbuf->refcount, 1); atomic_add(&buf->sbuf->refcount, 1);
@ -438,6 +462,16 @@ void gwbuf_add_buffer_object(
CHK_GWBUF(buf); CHK_GWBUF(buf);
newb = (buffer_object_t *)malloc(sizeof(buffer_object_t)); newb = (buffer_object_t *)malloc(sizeof(buffer_object_t));
ss_dassert(newb != NULL);
if (newb == NULL)
{
LOGIF(LE, (skygw_log_write_flush(
LOGFILE_ERROR,
"Error : Memory allocation failed due to %s.",
strerror(errno))));
return;
}
newb->bo_id = id; newb->bo_id = id;
newb->bo_data = data; newb->bo_data = data;
newb->bo_donefun_fp = donefun_fp; newb->bo_donefun_fp = donefun_fp;
@ -518,8 +552,15 @@ gwbuf_add_property(GWBUF *buf, char *name, char *value)
BUF_PROPERTY *prop; BUF_PROPERTY *prop;
if ((prop = malloc(sizeof(BUF_PROPERTY))) == NULL) if ((prop = malloc(sizeof(BUF_PROPERTY))) == NULL)
{
ss_dassert(prop != NULL);
LOGIF(LE, (skygw_log_write_flush(
LOGFILE_ERROR,
"Error : Memory allocation failed due to %s.",
strerror(errno))));
return 0; return 0;
}
prop->name = strdup(name); prop->name = strdup(name);
prop->value = strdup(value); prop->value = strdup(value);
spinlock_acquire(&buf->gwbuf_lock); spinlock_acquire(&buf->gwbuf_lock);

View File

@ -1891,7 +1891,6 @@ DCB_CALLBACK *cb, *nextcb;
int int
dcb_isvalid(DCB *dcb) dcb_isvalid(DCB *dcb)
{ {
DCB *ptr;
int rval = 0; int rval = 0;
if (dcb) if (dcb)

View File

@ -245,7 +245,7 @@ GWBUF *modutil_create_mysql_err_msg(
int affected_rows, int affected_rows,
int merrno, int merrno,
const char *statemsg, const char *statemsg,
const char * msg) const char *msg)
{ {
uint8_t *outbuf = NULL; uint8_t *outbuf = NULL;
uint8_t mysql_payload_size = 0; uint8_t mysql_payload_size = 0;
@ -259,6 +259,10 @@ GWBUF *modutil_create_mysql_err_msg(
const char *mysql_state = NULL; const char *mysql_state = NULL;
GWBUF *errbuf = NULL; GWBUF *errbuf = NULL;
if (statemsg == NULL || msg == NULL)
{
return NULL;
}
mysql_errno = (unsigned int)merrno; mysql_errno = (unsigned int)merrno;
mysql_error_msg = msg; mysql_error_msg = msg;
mysql_state = statemsg; mysql_state = statemsg;
@ -270,9 +274,6 @@ GWBUF *modutil_create_mysql_err_msg(
mysql_statemsg[0]='#'; mysql_statemsg[0]='#';
memcpy(mysql_statemsg+1, mysql_state, 5); memcpy(mysql_statemsg+1, mysql_state, 5);
if (msg != NULL) {
mysql_error_msg = msg;
}
mysql_payload_size = sizeof(field_count) + mysql_payload_size = sizeof(field_count) +
sizeof(mysql_err) + sizeof(mysql_err) +
sizeof(mysql_statemsg) + sizeof(mysql_statemsg) +
@ -283,8 +284,9 @@ GWBUF *modutil_create_mysql_err_msg(
ss_dassert(errbuf != NULL); ss_dassert(errbuf != NULL);
if (errbuf == NULL) if (errbuf == NULL)
{
return NULL; return NULL;
}
outbuf = GWBUF_DATA(errbuf); outbuf = GWBUF_DATA(errbuf);
/** write packet header and packet number */ /** write packet header and packet number */

View File

@ -1300,14 +1300,33 @@ static int gw_change_user(
backend->session->client->remote, backend->session->client->remote,
password_set, password_set,
""); "");
if (message == NULL)
{
LOGIF(LE, (skygw_log_write_flush(
LOGFILE_ERROR,
"Error : Creating error message failed.")));
rv = 0;
goto retblock;
}
/** TODO: Add custom message indicating that retry would probably help */ /** TODO: Add custom message indicating that retry would probably help */
buf = modutil_create_mysql_err_msg(1, 0, 1045, "28000", message); buf = modutil_create_mysql_err_msg(1, 0, 1045, "28000", message);
free(message);
if (buf == NULL)
{
LOGIF(LE, (skygw_log_write_flush(
LOGFILE_ERROR,
"Error : Creating buffer for error message failed.")));
rv = 0;
goto retblock;
}
/** Set flags that help router to identify session commans reply */ /** Set flags that help router to identify session commans reply */
gwbuf_set_type(buf, GWBUF_TYPE_MYSQL); gwbuf_set_type(buf, GWBUF_TYPE_MYSQL);
gwbuf_set_type(buf, GWBUF_TYPE_SESCMD_RESPONSE); gwbuf_set_type(buf, GWBUF_TYPE_SESCMD_RESPONSE);
gwbuf_set_type(buf, GWBUF_TYPE_RESPONSE_END); gwbuf_set_type(buf, GWBUF_TYPE_RESPONSE_END);
/** Create an incoming event for backend DCB */ /** Create an incoming event for backend DCB */
poll_add_epollin_event_to_dcb(backend, buf); poll_add_epollin_event_to_dcb(backend, gwbuf_clone(buf));
gwbuf_free(buf);
rv = 0; rv = 0;
} else { } else {
rv = gw_send_change_user_to_backend(database, username, client_sha1, backend_protocol); rv = gw_send_change_user_to_backend(database, username, client_sha1, backend_protocol);
@ -1318,6 +1337,8 @@ static int gw_change_user(
strcpy(current_session->db, database); strcpy(current_session->db, database);
memcpy(current_session->client_sha1, client_sha1, sizeof(current_session->client_sha1)); memcpy(current_session->client_sha1, client_sha1, sizeof(current_session->client_sha1));
} }
retblock:
gwbuf_free(queue); gwbuf_free(queue);
return rv; return rv;

View File

@ -2106,19 +2106,34 @@ char *create_auth_fail_str(
else else
db_len = 0; db_len = 0;
if (db_len>0) if (db_len > 0)
{
ferrstr = "Access denied for user '%s'@'%s' (using password: %s) to database '%s'"; ferrstr = "Access denied for user '%s'@'%s' (using password: %s) to database '%s'";
}
else else
{
ferrstr = "Access denied for user '%s'@'%s' (using password: %s)"; ferrstr = "Access denied for user '%s'@'%s' (using password: %s)";
}
errstr = (char *)malloc(strlen(username)+strlen(ferrstr)+strlen(hostaddr)+strlen("YES")-6 + db_len + ((db_len > 0) ? (strlen(" to database ") +2) : 0) + 1); errstr = (char *)malloc(strlen(username)+strlen(ferrstr)+strlen(hostaddr)+strlen("YES")-6 + db_len + ((db_len > 0) ? (strlen(" to database ") +2) : 0) + 1);
if (errstr != NULL) { if (errstr == NULL)
if (db_len>0) {
sprintf(errstr, ferrstr, username, hostaddr, (*sha1 == '\0' ? "NO" : "YES"), db); LOGIF(LE, (skygw_log_write_flush(
else LOGFILE_ERROR,
sprintf(errstr, ferrstr, username, hostaddr, (*sha1 == '\0' ? "NO" : "YES")); "Error : Memory allocation failed due to %s.",
strerror(errno))));
goto retblock;
} }
if (db_len > 0)
{
sprintf(errstr, ferrstr, username, hostaddr, (*sha1 == '\0' ? "NO" : "YES"), db);
}
else
{
sprintf(errstr, ferrstr, username, hostaddr, (*sha1 == '\0' ? "NO" : "YES"));
}
retblock:
return errstr; return errstr;
} }

View File

@ -654,7 +654,7 @@ DCB* backend_dcb;
* @param instance The router instance * @param instance The router instance
* @param router_session The router session returned from the newSession call * @param router_session The router session returned from the newSession call
* @param queue The queue of data buffers to route * @param queue The queue of data buffers to route
* @return The number of bytes sent * @return if succeed 1, otherwise 0
*/ */
static int static int
routeQuery(ROUTER *instance, void *router_session, GWBUF *queue) routeQuery(ROUTER *instance, void *router_session, GWBUF *queue)
@ -697,20 +697,22 @@ routeQuery(ROUTER *instance, void *router_session, GWBUF *queue)
"Error : Failed to route MySQL command %d to backend " "Error : Failed to route MySQL command %d to backend "
"server.", "server.",
mysql_command))); mysql_command)));
rc = 0;
goto return_rc; goto return_rc;
} }
switch(mysql_command) { switch(mysql_command) {
case MYSQL_COM_CHANGE_USER: case MYSQL_COM_CHANGE_USER:
rc = backend_dcb->func.auth( rc = backend_dcb->func.auth(
backend_dcb, backend_dcb,
NULL, NULL,
backend_dcb->session, backend_dcb->session,
queue); queue);
break; break;
default:
rc = backend_dcb->func.write(backend_dcb, queue); default:
break; rc = backend_dcb->func.write(backend_dcb, queue);
break;
} }
CHK_PROTOCOL(((MySQLProtocol*)backend_dcb->protocol)); CHK_PROTOCOL(((MySQLProtocol*)backend_dcb->protocol));

View File

@ -3829,7 +3829,6 @@ static bool route_session_write(
{ {
succp = false; succp = false;
} }
} }
} }
rses_end_locked_router_action(router_cli_ses); rses_end_locked_router_action(router_cli_ses);
@ -3842,6 +3841,12 @@ static bool route_session_write(
succp = false; succp = false;
goto return_succp; goto return_succp;
} }
if (router_cli_ses->rses_nbackends <= 0)
{
succp = false;
goto return_succp;
}
/** /**
* Additional reference is created to querybuf to * Additional reference is created to querybuf to
* prevent it from being released before properties * prevent it from being released before properties
@ -3909,6 +3914,10 @@ static bool route_session_write(
} }
} }
} }
else
{
succp = false;
}
} }
/** Unlock router session */ /** Unlock router session */
rses_end_locked_router_action(router_cli_ses); rses_end_locked_router_action(router_cli_ses);