From 4ae2b9b72dd4fce3d7a7778222d1c39abbb564a4 Mon Sep 17 00:00:00 2001 From: Haru Date: Tue, 20 Jan 2015 04:36:08 +0100 Subject: Minor fixes and tweaks suggested by cppcheck - Variable scopes reduced - Parenthesized ambiguous expressions - Removed or added NULL checks where (un)necessary - Corrected format strings - Fixed typos potentially leading to bugs Signed-off-by: Haru --- src/common/HPM.c | 20 ++++++++++---------- src/common/core.c | 13 ++++++------- src/common/db.c | 12 +++++------- src/common/malloc.c | 5 ++--- src/common/socket.c | 41 +++++++++++++++------------------------- src/common/sql.c | 53 ++++++++++++++++++++++------------------------------ src/common/strlib.c | 11 +++++------ src/common/sysinfo.c | 2 +- src/common/utils.c | 5 +++-- 9 files changed, 69 insertions(+), 93 deletions(-) (limited to 'src/common') diff --git a/src/common/HPM.c b/src/common/HPM.c index 10208cdad..25ca5441d 100644 --- a/src/common/HPM.c +++ b/src/common/HPM.c @@ -95,7 +95,6 @@ struct hplugin *hplugin_create(void) { } #define HPM_POP(x) { #x , x } bool hplugin_populate(struct hplugin *plugin, const char *filename) { - void **Link; struct { const char* name; void *Ref; @@ -113,7 +112,8 @@ bool hplugin_populate(struct hplugin *plugin, const char *filename) { int i, length = ARRAYLENGTH(ToLink); for(i = 0; i < length; i++) { - if( !( Link = plugin_import(plugin->dll, ToLink[i].name,void **) ) ) { + void **Link; + if (!( Link = plugin_import(plugin->dll, ToLink[i].name,void **))) { ShowWarning("HPM:plugin_load: failed to retrieve '%s' for '"CL_WHITE"%s"CL_RESET"', skipping...\n", ToLink[i].name, filename); HPM->unload(plugin); return false; @@ -268,7 +268,7 @@ struct hplugin *hplugin_load(const char* filename) { } void hplugin_unload(struct hplugin* plugin) { - unsigned int i = plugin->idx, cursor = 0; + unsigned int i = plugin->idx; if( plugin->filename ) aFree(plugin->filename); @@ -277,7 +277,8 @@ void hplugin_unload(struct hplugin* plugin) { /* TODO: for manual packet unload */ /* - Go through known packets and unlink any belonging to the plugin being removed */ aFree(plugin); - if( !HPM->off ) { + if (!HPM->off) { + int cursor = 0; HPM->plugins[i] = NULL; for(i = 0; i < HPM->plugin_count; i++) { if( HPM->plugins[i] == NULL ) @@ -838,14 +839,13 @@ void hpm_init(void) { #endif return; } -void hpm_memdown(void) { - unsigned int i; - +void hpm_memdown(void) +{ /* this memory is handled outside of the server's memory manager and thus cleared after memory manager goes down */ - if (HPM->fnames) - { - for( i = 0; i < HPM->fnamec; i++ ) { + if (HPM->fnames) { + unsigned int i; + for (i = 0; i < HPM->fnamec; i++) { free(HPM->fnames[i].name); } free(HPM->fnames); diff --git a/src/common/core.c b/src/common/core.c index 6ad971c86..8bf381589 100644 --- a/src/common/core.c +++ b/src/common/core.c @@ -240,7 +240,7 @@ static CMDLINEARG(help) } else { *altname = '\0'; } - snprintf(paramnames, sizeof(paramnames), "%s%s%s", data->name, altname, data->options&CMDLINE_OPT_PARAM ? " " : ""); + snprintf(paramnames, sizeof(paramnames), "%s%s%s", data->name, altname, (data->options&CMDLINE_OPT_PARAM) ? " " : ""); ShowInfo(" %-30s %s [%s]\n", paramnames, data->help ? data->help : "", cmdline->arg_source(data)); } return false; @@ -439,12 +439,11 @@ int main (int argc, char **argv) { sockt->init(); do_init(argc,argv); - {// Main runtime cycle - int next; - while (runflag != CORE_ST_STOP) { - next = timer->perform(timer->gettick_nocache()); - sockt->perform(next); - } + + // Main runtime cycle + while (runflag != CORE_ST_STOP) { + int next = timer->perform(timer->gettick_nocache()); + sockt->perform(next); } console->final(); diff --git a/src/common/db.c b/src/common/db.c index 044df19aa..69e2333a9 100644 --- a/src/common/db.c +++ b/src/common/db.c @@ -497,7 +497,6 @@ static void db_rebalance_erase(DBNode *node, DBNode **root) DBNode *y = node; DBNode *x = NULL; DBNode *x_parent = NULL; - DBNode *w; DB_COUNTSTAT(db_rebalance_erase); // Select where to change the tree @@ -565,6 +564,7 @@ static void db_rebalance_erase(DBNode *node, DBNode **root) // Restore the RED-BLACK properties if (y->color != RED) { while (x != *root && (x == NULL || x->color == BLACK)) { + DBNode *w; if (x == x_parent->left) { w = x_parent->right; if (w->color == RED) { @@ -1526,7 +1526,6 @@ static bool db_obj_exists(DBMap* self, DBKey key) { DBMap_impl* db = (DBMap_impl*)self; DBNode *node; - int c; bool found = false; DB_COUNTSTAT(db_exists); @@ -1548,7 +1547,7 @@ static bool db_obj_exists(DBMap* self, DBKey key) db_free_lock(db); node = db->ht[db->hash(key, db->maxlen)%HASH_SIZE]; while (node) { - c = db->cmp(key, node->key, db->maxlen); + int c = db->cmp(key, node->key, db->maxlen); if (c == 0) { if (!(node->deleted)) { db->cache = node; @@ -1577,7 +1576,6 @@ static DBData* db_obj_get(DBMap* self, DBKey key) { DBMap_impl* db = (DBMap_impl*)self; DBNode *node; - int c; DBData *data = NULL; DB_COUNTSTAT(db_get); @@ -1600,7 +1598,7 @@ static DBData* db_obj_get(DBMap* self, DBKey key) db_free_lock(db); node = db->ht[db->hash(key, db->maxlen)%HASH_SIZE]; while (node) { - c = db->cmp(key, node->key, db->maxlen); + int c = db->cmp(key, node->key, db->maxlen); if (c == 0) { if (!(node->deleted)) { data = &node->data; @@ -1970,7 +1968,7 @@ static int db_obj_remove(DBMap* self, DBKey key, DBData *out_data) DBMap_impl* db = (DBMap_impl*)self; DBNode *node; unsigned int hash; - int c = 0, retval = 0; + int retval = 0; DB_COUNTSTAT(db_remove); if (db == NULL) return 0; // nullpo candidate @@ -1988,7 +1986,7 @@ static int db_obj_remove(DBMap* self, DBKey key, DBData *out_data) db_free_lock(db); hash = db->hash(key, db->maxlen)%HASH_SIZE; for(node = db->ht[hash]; node; ){ - c = db->cmp(key, node->key, db->maxlen); + int c = db->cmp(key, node->key, db->maxlen); if (c == 0) { if (!(node->deleted)) { if (db->cache == node) diff --git a/src/common/malloc.c b/src/common/malloc.c index 43fbe4ea1..c647dc18f 100644 --- a/src/common/malloc.c +++ b/src/common/malloc.c @@ -124,8 +124,7 @@ void aFree_(void *p, const char *file, int line, const char *func) // ShowMessage("%s:%d: in func %s: aFree %p\n",file,line,func,p); if (p) FREE(p, file, line, func); - - p = NULL; + //p = NULL; } @@ -487,13 +486,13 @@ void mfree_(void *ptr, const char *file, int line, const char *func) { /* Allocating blocks */ static struct block* block_malloc(unsigned short hash) { - int i; struct block *p; if(hash_unfill[0] != NULL) { /* Space for the block has already been secured */ p = hash_unfill[0]; hash_unfill[0] = hash_unfill[0]->unfill_next; } else { + int i; /* Newly allocated space for the block */ p = (struct block*)MALLOC(sizeof(struct block) * (BLOCK_ALLOC), __FILE__, __LINE__, __func__ ); memmgr_usage_bytes_t += sizeof(struct block) * (BLOCK_ALLOC); diff --git a/src/common/socket.c b/src/common/socket.c index 600666d21..9fe08e6f1 100644 --- a/src/common/socket.c +++ b/src/common/socket.c @@ -624,7 +624,6 @@ static int create_session(int fd, RecvFunc func_recv, SendFunc func_send, ParseF static void delete_session(int fd) { if( sockt->session_isValid(fd) ) { - unsigned int i; #ifdef SHOW_SERVER_STATS socket_data_qi -= session[fd]->rdata_size - session[fd]->rdata_pos; socket_data_qo -= session[fd]->wdata_size; @@ -633,8 +632,8 @@ static void delete_session(int fd) aFree(session[fd]->wdata); if( session[fd]->session_data ) aFree(session[fd]->session_data); - if( session[fd]->hdata ) - { + if (session[fd]->hdata) { + unsigned int i; for(i = 0; i < session[fd]->hdatac; i++) { if( session[fd]->hdata[i]->flag.free ) { aFree(session[fd]->hdata[i]->data); @@ -1249,38 +1248,31 @@ int socket_getips(uint32* ips, int max) #ifdef WIN32 { char fullhost[255]; - u_long** a; - struct hostent* hent; // XXX This should look up the local IP addresses in the registry // instead of calling gethostbyname. However, the way IP addresses // are stored in the registry is annoyingly complex, so I'll leave // this as T.B.D. [Meruru] - if( gethostname(fullhost, sizeof(fullhost)) == SOCKET_ERROR ) - { + if (gethostname(fullhost, sizeof(fullhost)) == SOCKET_ERROR) { ShowError("socket_getips: No hostname defined!\n"); return 0; - } - else - { - hent = gethostbyname(fullhost); + } else { + u_long** a; + struct hostent *hent =gethostbyname(fullhost); if( hent == NULL ){ ShowError("socket_getips: Cannot resolve our own hostname to an IP address\n"); return 0; } a = (u_long**)hent->h_addr_list; - for( ; a[num] != NULL && num < max; ++num) + for (; num < max && a[num] != NULL; ++num) ips[num] = (uint32)ntohl(*a[num]); } } #else // not WIN32 { - int pos; int fd; char buf[2*16*sizeof(struct ifreq)]; struct ifconf ic; - struct ifreq* ir; - struct sockaddr_in* a; u_long ad; fd = sSocket(AF_INET, SOCK_STREAM, 0); @@ -1291,21 +1283,18 @@ int socket_getips(uint32* ips, int max) // interfaces than will fit in the buffer ic.ifc_len = sizeof(buf); ic.ifc_buf = buf; - if( sIoctl(fd, SIOCGIFCONF, &ic) == -1 ) - { + if (sIoctl(fd, SIOCGIFCONF, &ic) == -1) { ShowError("socket_getips: SIOCGIFCONF failed!\n"); sClose(fd); return 0; - } - else - { - for( pos=0; pos < ic.ifc_len && num < max; ) - { - ir = (struct ifreq*)(buf+pos); - a = (struct sockaddr_in*) &(ir->ifr_addr); - if( a->sin_family == AF_INET ){ + } else { + int pos; + for (pos = 0; pos < ic.ifc_len && num < max; ) { + struct ifreq *ir = (struct ifreq*)(buf+pos); + struct sockaddr_in *a = (struct sockaddr_in*) &(ir->ifr_addr); + if (a->sin_family == AF_INET) { ad = ntohl(a->sin_addr.s_addr); - if( ad != INADDR_LOOPBACK && ad != INADDR_ANY ) + if (ad != INADDR_LOOPBACK && ad != INADDR_ANY) ips[num++] = (uint32)ad; } #if (defined(BSD) && BSD >= 199103) || defined(_AIX) || defined(__APPLE__) diff --git a/src/common/sql.c b/src/common/sql.c index f0b2365a4..aec2ae93d 100644 --- a/src/common/sql.c +++ b/src/common/sql.c @@ -816,10 +816,8 @@ int SqlStmt_NextRow(SqlStmt* self) int err; size_t i; size_t cols; - MYSQL_BIND* column; - unsigned long length; - if( self == NULL ) + if (self == NULL) return SQL_ERROR; // bind columns @@ -829,30 +827,27 @@ int SqlStmt_NextRow(SqlStmt* self) err = mysql_stmt_fetch(self->stmt);// fetch row // check for errors - if( err == MYSQL_NO_DATA ) + if (err == MYSQL_NO_DATA) return SQL_NO_DATA; #if defined(MYSQL_DATA_TRUNCATED) // MySQL 5.0/5.1 defines and returns MYSQL_DATA_TRUNCATED [FlavioJS] - if( err == MYSQL_DATA_TRUNCATED ) - { + if (err == MYSQL_DATA_TRUNCATED) { my_bool truncated; - if( !self->bind_columns ) - { + if (!self->bind_columns) { ShowSQL("DB error - data truncated (unknown source, columns are not bound)\n"); return SQL_ERROR; } // find truncated column cols = SQL->StmtNumColumns(self); - for( i = 0; i < cols; ++i ) - { - column = &self->columns[i]; + for (i = 0; i < cols; ++i) { + MYSQL_BIND *column = &self->columns[i]; column->error = &truncated; mysql_stmt_fetch_column(self->stmt, column, (unsigned int)i, 0); column->error = NULL; - if( truncated ) - {// report truncated column + if (truncated) { + // report truncated column SqlStmt_P_ShowDebugTruncatedColumn(self, i); return SQL_ERROR; } @@ -861,8 +856,7 @@ int SqlStmt_NextRow(SqlStmt* self) return SQL_ERROR; } #endif - if( err ) - { + if (err) { ShowSQL("DB error - %s\n", mysql_stmt_error(self->stmt)); hercules_mysql_error_handler(mysql_stmt_errno(self->stmt)); return SQL_ERROR; @@ -870,30 +864,28 @@ int SqlStmt_NextRow(SqlStmt* self) // propagate column lengths and clear unused parts of string/enum/blob buffers cols = SQL->StmtNumColumns(self); - for( i = 0; i < cols; ++i ) - { - length = self->column_lengths[i].length; - column = &self->columns[i]; + for (i = 0; i < cols; ++i) { + unsigned long length = self->column_lengths[i].length; + MYSQL_BIND *column = &self->columns[i]; #if !defined(MYSQL_DATA_TRUNCATED) // MySQL 4.1/(below?) returns success even if data is truncated, so we test truncation manually [FlavioJS] - if( column->buffer_length < length ) - {// report truncated column - if( column->buffer_type == MYSQL_TYPE_STRING || column->buffer_type == MYSQL_TYPE_BLOB ) - {// string/enum/blob column + if (column->buffer_length < length) { + // report truncated column + if (column->buffer_type == MYSQL_TYPE_STRING || column->buffer_type == MYSQL_TYPE_BLOB) { + // string/enum/blob column SqlStmt_P_ShowDebugTruncatedColumn(self, i); return SQL_ERROR; } // FIXME numeric types and null [FlavioJS] } #endif - if( self->column_lengths[i].out_length ) + if (self->column_lengths[i].out_length) *self->column_lengths[i].out_length = (uint32)length; - if( column->buffer_type == MYSQL_TYPE_STRING ) - {// clear unused part of the string/enum buffer (and null-terminate) + if (column->buffer_type == MYSQL_TYPE_STRING) { + // clear unused part of the string/enum buffer (and null-terminate) memset((char*)column->buffer + length, 0, column->buffer_length - length + 1); - } - else if( column->buffer_type == MYSQL_TYPE_BLOB && length < column->buffer_length ) - {// clear unused part of the blob buffer + } else if (column->buffer_type == MYSQL_TYPE_BLOB && length < column->buffer_length) { + // clear unused part of the blob buffer memset((char*)column->buffer + length, 0, column->buffer_length - length); } } @@ -958,7 +950,6 @@ void hercules_mysql_error_handler(unsigned int ecode) { } } void Sql_inter_server_read(const char* cfgName, bool first) { - int i; char line[1024], w1[1024], w2[1024]; FILE* fp; @@ -973,7 +964,7 @@ void Sql_inter_server_read(const char* cfgName, bool first) { } while (fgets(line, sizeof(line), fp)) { - i = sscanf(line, "%1023[^:]: %1023[^\r\n]", w1, w2); + int i = sscanf(line, "%1023[^:]: %1023[^\r\n]", w1, w2); if (i != 2) continue; diff --git a/src/common/strlib.c b/src/common/strlib.c index 8b63b4161..b5fcff576 100644 --- a/src/common/strlib.c +++ b/src/common/strlib.c @@ -946,7 +946,6 @@ bool sv_readdb(const char* directory, const char* filename, char delim, int minc char** fields; // buffer for fields ([0] is reserved) int columns, fields_length; char path[1024], line[1024]; - char* match; snprintf(path, sizeof(path), "%s/%s", directory, filename); @@ -962,9 +961,11 @@ bool sv_readdb(const char* directory, const char* filename, char delim, int minc // process rows one by one while( fgets(line, sizeof(line), fp) ) { + char *match; lines++; - if( ( match = strstr(line, "//") ) != NULL ) {// strip comments + if ((match = strstr(line, "//") ) != NULL) { + // strip comments match[0] = 0; } @@ -1039,13 +1040,11 @@ int StringBuf_Printf(StringBuf *self, const char *fmt, ...) { /// Appends the result of vprintf to the StringBuf int StringBuf_Vprintf(StringBuf* self, const char* fmt, va_list ap) { - int n, off; - size_t size; - for(;;) { va_list apcopy; + int n, off; /* Try to print in the allocated space. */ - size = self->max_ - (self->ptr_ - self->buf_); + size_t size = self->max_ - (self->ptr_ - self->buf_); va_copy(apcopy, ap); n = vsnprintf(self->ptr_, size, fmt, apcopy); va_end(apcopy); diff --git a/src/common/sysinfo.c b/src/common/sysinfo.c index ee8ff504a..a1dbed4d1 100644 --- a/src/common/sysinfo.c +++ b/src/common/sysinfo.c @@ -319,12 +319,12 @@ bool sysinfo_svn_get_revision(char **out) { bool sysinfo_git_get_revision(char **out) { // Only include Git support if we detected it, or we're on MSVC #if !defined(SYSINFO_VCSTYPE) || SYSINFO_VCSTYPE == VCSTYPE_GIT || SYSINFO_VCSTYPE == VCSTYPE_UNKNOWN - FILE *fp; char ref[128], filepath[128], line[128]; strcpy(ref, "HEAD"); while (*ref) { + FILE *fp; snprintf(filepath, sizeof(filepath), ".git/%s", ref); if ((fp = fopen(filepath, "r")) != NULL) { if (fgets(line, sizeof(line)-1, fp) == NULL) { diff --git a/src/common/utils.c b/src/common/utils.c index 5ede86296..c168bd74e 100644 --- a/src/common/utils.c +++ b/src/common/utils.c @@ -95,8 +95,9 @@ void ShowDump(const void *buffer, size_t length) { static char* checkpath(char *path, const char *srcpath) { // just make sure the char*path is not const - char *p=path; - if(NULL!=path && NULL!=srcpath) + char *p = path; + if (NULL == path || NULL == srcpath) + return path; while(*srcpath) { if (*srcpath=='/') { *p++ = '\\'; -- cgit v1.2.3-60-g2f50