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 <haru@dotalux.com>

9 files changed, 69 insertions, 93 deletions

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 ? " <name>" : "");
+		snprintf(paramnames, sizeof(paramnames), "%s%s%s", data->name, altname, (data->options&CMDLINE_OPT_PARAM) ? " <name>" : "");
 		ShowInfo("  %-30s %s [%s]\n", paramnames, data->help ? data->help : "<no description provided>", 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++ = '\\';