Cppcheck

Fix typo in race bonus fix : race<RC_NONE_ && race<RC_MAX
Fix bugreport:8418 => clif_instance_changestatus possible mapcrash
Reduce lot of variable scope and fix unsafe scanf

src/common/socket.c

@@ -1089,9 +1089,9 @@ int access_ipmask(const char* str, AccessControl* acc)
 		unsigned int a[4];
 		unsigned int m[4];
 		int n;
-		if( ((n=sscanf(str,"%u.%u.%u.%u/%u.%u.%u.%u",a,a+1,a+2,a+3,m,m+1,m+2,m+3)) != 8 && // not an ip + standard mask
-				(n=sscanf(str,"%u.%u.%u.%u/%u",a,a+1,a+2,a+3,m)) != 5 && // not an ip + bit mask
-				(n=sscanf(str,"%u.%u.%u.%u",a,a+1,a+2,a+3)) != 4 ) || // not an ip
+		if( ((n=sscanf(str,"%3u.%3u.%3u.%3u/%3u.%3u.%3u.%3u",a,a+1,a+2,a+3,m,m+1,m+2,m+3)) != 8 && // not an ip + standard mask
+				(n=sscanf(str,"%3u.%3u.%3u.%3u/%3u",a,a+1,a+2,a+3,m)) != 5 && // not an ip + bit mask
+				(n=sscanf(str,"%3u.%3u.%3u.%3u",a,a+1,a+2,a+3)) != 4 ) || // not an ip
 				a[0] > 255 || a[1] > 255 || a[2] > 255 || a[3] > 255 || // invalid ip
 				(n == 8 && (m[0] > 255 || m[1] > 255 || m[2] > 255 || m[3] > 255)) || // invalid standard mask
 				(n == 5 && m[0] > 32) ){ // invalid bit mask
@@ -1139,7 +1139,7 @@ int socket_config_read(const char* cfgName)
 	{
 		if(line[0] == '/' && line[1] == '/')
 			continue;
-		if(sscanf(line, "%[^:]: %[^\r\n]", w1, w2) != 2)
+		if(sscanf(line, "%1023[^:]: %1023[^\r\n]", w1, w2) != 2)
 			continue;
 
 		if (!strcmpi(w1, "stall_time")) {

src/common/timer.c

@@ -457,7 +457,6 @@ void split_time(int timein, int* year, int* month, int* day, int* hour, int* min
  */
 double solve_time(char* modif_p) {
 	double totaltime = 0;
-	int value = 0;
 	struct tm then_tm;
 	time_t now = time(NULL);
 	time_t then = now;
@@ -466,7 +465,7 @@ double solve_time(char* modif_p) {
 	nullpo_retr(0,modif_p);
 
 	while (modif_p[0] != '\0') {
-		value = atoi(modif_p);
+		int value = atoi(modif_p);
 		if (value == 0)
 			modif_p++;
 		else {

src/login/login.c

@@ -345,7 +345,7 @@ int login_lan_config_read(const char *lancfgName)
 		if ((line[0] == '/' && line[1] == '/') || line[0] == '\n' || line[1] == '\n')
 			continue;
 
-		if(sscanf(line,"%[^:]: %[^:]:%[^:]:%[^\r\n]", w1, w2, w3, w4) != 4)
+		if(sscanf(line,"%63[^:]: %63[^:]:%63[^:]:%63[^\r\n]", w1, w2, w3, w4) != 4)
 		{
 			ShowWarning("Error syntax of configuration file %s in line %d.\n", lancfgName, line_num);
 			continue;
@@ -922,21 +922,18 @@ int parse_fromchar(int fd){
 			if (RFIFOREST(fd) < 4 || RFIFOREST(fd) < RFIFOW(fd,2))
 				return 0;
 			else{
-				struct online_login_data *p;
-				int aid;
 				uint32 i, users;
 				online_db->foreach(online_db, online_db_setoffline, id); //Set all chars from this char-server offline first
 				users = RFIFOW(fd,4);
 				for (i = 0; i < users; i++) {
-					aid = RFIFOL(fd,6+i*4);
-					p = idb_ensure(online_db, aid, create_online_user);
+					int aid = RFIFOL(fd,6+i*4);
+					struct online_login_data *p = idb_ensure(online_db, aid, create_online_user);
 					p->char_server = id;
 					if (p->waiting_disconnect != INVALID_TIMER){
 						delete_timer(p->waiting_disconnect, waiting_disconnect_timer);
 						p->waiting_disconnect = INVALID_TIMER;
 					}
 				}
-
 				RFIFOSKIP(fd,RFIFOW(fd,2));
 			}
 		break;
@@ -1773,7 +1770,7 @@ int login_config_read(const char* cfgName)
 		if (line[0] == '/' && line[1] == '/')
 			continue;
 
-		if (sscanf(line, "%[^:]: %[^\r\n]", w1, w2) < 2)
+		if (sscanf(line, "%1023[^:]: %1023[^\r\n]", w1, w2) < 2)
 			continue;
 
 		if(!strcmpi(w1,"timestamp_format"))
@@ -1835,7 +1832,7 @@ int login_config_read(const char* cfgName)
 		else if(!strcmpi(w1, "client_hash")) {
 			int group = 0;
 			char md5[33];
-			if (sscanf(w2, "%d, %32s", &group, md5) == 2) {
+			if (sscanf(w2, "%3d, %32s", &group, md5) == 2) {
 				struct client_hash_node *nnode;
 				int i;
 				CREATE(nnode, struct client_hash_node, 1);
@@ -1844,7 +1841,7 @@ int login_config_read(const char* cfgName)
 					unsigned int byte;
 					memcpy(buf, &md5[i], 2);
 					buf[2] = 0;
-					sscanf(buf, "%x", &byte);
+					sscanf(buf, "%2x", &byte);
 					nnode->hash[i / 2] = (uint8)(byte & 0xFF);
 				}
 				nnode->group_id = group;

src/map/battle.c

@@ -3922,7 +3922,6 @@ struct Damage battle_calc_defense_reduction(struct Damage wd, struct block_list
 	struct status_change *tsc = status_get_sc(target);
 	struct status_data *sstatus = status_get_status_data(src);
 	struct status_data *tstatus = status_get_status_data(target);
-	int i;
 
 	//Defense reduction
 	short vit_def;
@@ -3932,11 +3931,9 @@ struct Damage battle_calc_defense_reduction(struct Damage wd, struct block_list
 	if( tsc && tsc->data[SC_ASSUMPTIO] )
 		def1 <<= 1; // only eDEF is doubled
 #endif
-	if( sd )
-	{
-		i = sd->ignore_def_by_race[tstatus->race] + sd->ignore_def_by_race[RC_ALL];
-		if( i )
-		{
+	if( sd ) {
+		int i = sd->ignore_def_by_race[tstatus->race] + sd->ignore_def_by_race[RC_ALL];
+		if( i ) {
 			if( i > 100 ) i = 100;
 			def1 -= def1 * i / 100;
 			def2 -= def2 * i / 100;

src/map/channel.c

@@ -117,14 +117,13 @@ int channel_delete(struct Channel *channel) {
  * -3 : sd banned
  */
 int channel_join(struct Channel *channel, struct map_session_data *sd) {
-	char output[128];
-
 	if(!channel || !sd)
 		return -1;
 	if(channel_haspc(channel,sd)==1)
 		return -2;
 
 	if(channel_haspcbanned(channel,sd)==1){
+		char output[128];
 		sprintf(output, msg_txt(sd,1438),channel->name); //You're currently banned from the '%s' channel.
 		clif_displaymessage(sd->fd, output);
 		return -3;
@@ -211,10 +210,8 @@ int channel_ajoin(struct guild *g){
 int channel_gjoin(struct map_session_data *sd, int flag){
 	struct Channel *channel;
 	struct guild *g;
-	int i;
 
 	if(!sd) return -1;
-
 	g = sd->guild;
 	if(!g) return -2;
 
@@ -228,6 +225,7 @@ int channel_gjoin(struct map_session_data *sd, int flag){
 		channel_join(channel,sd);	//join our guild chat
 	}
 	if(flag&2){
+		int i;
 		for (i = 0; i < MAX_GUILDALLIANCE; i++){
 			struct guild *ag; //allied guld
 			struct guild_alliance *ga = &g->alliance[i]; //guild alliance
@@ -305,8 +303,8 @@ int channel_pcquit(struct map_session_data *sd, int type){
 			channel_clean(g->channel,sd,0); //leave guild chan
 		}
 		if(type&2){
-			struct guild *ag; //allied guild
 			for (i = 0; i < MAX_GUILDALLIANCE; i++) { //leave all alliance chan
+				struct guild *ag; //allied guild
 				if( g->alliance[i].guild_id && (ag = guild_search(g->alliance[i].guild_id) ) ) {
 					if(channel_haspc(ag->channel,sd) == 1)
 						channel_clean(ag->channel,sd,0);
@@ -538,7 +536,6 @@ int channel_display_list(struct map_session_data *sd, char *options){
  *  -1 : fail
  */
 int channel_pccreate(struct map_session_data *sd, char *chname, char *chpass){
-	struct Channel *channel;
 	char output[128];
 	int8 res;
 
@@ -547,7 +544,7 @@ int channel_pccreate(struct map_session_data *sd, char *chname, char *chpass){
 
 	res = channel_chk(chname,chpass,7);
 	if(res==0){ //success
-		channel = channel_create(chname + 1,chpass,0,CHAN_TYPE_PRIVATE,sd->status.char_id);
+		struct Channel *channel = channel_create(chname + 1,chpass,0,CHAN_TYPE_PRIVATE,sd->status.char_id);
 		channel_join(channel,sd);
 		if( !( channel->opt & CHAN_OPT_ANNOUNCE_JOIN ) ) {
 			sprintf(output, msg_txt(sd,1403),chname); // You're now in the '%s' channel.
@@ -931,7 +928,7 @@ int channel_pcsetopt(struct map_session_data *sd, char *chname, const char *opti
 		return -1;
 	}
 
-	if(!option || option == '\0' ) {
+	if(!option || option[0] == '\0' ) {
 		clif_displaymessage(sd->fd, msg_txt(sd,1446));// You need to input an option.
 		return -1;
 	}
@@ -1152,8 +1149,7 @@ int do_init_channel(void) {
 void do_final_channel(void) {
 	DBIterator *iter;
 	struct Channel *channel;
-	int i=0;
-
+	
 	//delete all in remaining chan db
 	iter = db_iterator(channel_db);
 	for( channel = dbi_first(iter); dbi_exists(iter); channel = dbi_next(iter) ) {
@@ -1165,6 +1161,7 @@ void do_final_channel(void) {
 
 	//delete all color thing
 	if( Channel_Config.colors_count ) {
+		int i=0;
 		for(i = 0; i < Channel_Config.colors_count; i++) {
 			aFree(Channel_Config.colors_name[i]);
 		}

src/map/clif.c

@@ -15719,7 +15719,7 @@ void clif_instance_create(struct map_session_data *sd, const char *name, int num
 #if PACKETVER >= 20071128
 	unsigned char buf[65];
 
-	nullpo_retv(sd);
+	if(!sd) return;
 
 	WBUFW(buf,0) = 0x2cb;
 	safestrncpy( WBUFP(buf,2), name, 62 );
@@ -15738,7 +15738,7 @@ void clif_instance_changewait(struct map_session_data *sd, int num, int flag)
 #if PACKETVER >= 20071128
 	unsigned char buf[4];
 
-	nullpo_retv(sd);
+	if(!sd) return;
 
 	WBUFW(buf,0) = 0x2cc;
 	WBUFW(buf,2) = num;
@@ -15776,7 +15776,7 @@ void clif_instance_changestatus(struct map_session_data *sd, int type, unsigned
 #if PACKETVER >= 20071128
 	unsigned char buf[10];
 
-	nullpo_retv(sd);
+	if(!sd) return;
 
 	WBUFW(buf,0) = 0x2ce;
 	WBUFL(buf,2) = type;

src/map/guild.c

@@ -887,10 +887,11 @@ void guild_retrieveitembound(int char_id,int aid,int guild_id)
 	TBL_PC *sd = map_id2sd(aid);
 	if(sd){ //Character is online
 		int idxlist[MAX_INVENTORY];
-		int j,i;
+		int j;
 		j = pc_bound_chk(sd,2,idxlist);
 		if(j) {
 			struct guild_storage* stor = guild2storage(sd->status.guild_id);
+			int i;
 			for(i=0;i<j;i++) { //Loop the matching items, guild_storage_additem takes care of opening storage
 				if(stor)
 					guild_storage_additem(sd,stor,&sd->status.inventory[idxlist[i]],sd->status.inventory[idxlist[i]].amount);
@@ -902,9 +903,9 @@ void guild_retrieveitembound(int char_id,int aid,int guild_id)
 	else { //Character is offline, ask char server to do the job
 		struct guild_storage* stor = guild2storage2(guild_id);
 		struct guild *g = guild_search(guild_id);
-		int i;
 		nullpo_retv(g);
 		if(stor && stor->storage_status == 1) { //Someone is in guild storage, close them
+			int i;
 			for(i=0; i<g->max_member; i++){
 				TBL_PC *pl_sd = g->member[i].sd;
 				if(pl_sd && pl_sd->state.storage_flag == 2)
@@ -1120,7 +1121,6 @@ int guild_change_notice(struct map_session_data *sd,int guild_id,const char *mes
 int guild_notice_changed(int guild_id,const char *mes1,const char *mes2)
 {
 	int i;
-	struct map_session_data *sd;
 	struct guild *g=guild_search(guild_id);
 	if(g==NULL)
 		return 0;
@@ -1129,7 +1129,8 @@ int guild_notice_changed(int guild_id,const char *mes1,const char *mes2)
 	memcpy(g->mes2,mes2,MAX_GUILDMES2);
 
 	for(i=0;i<g->max_member;i++){
-		if((sd=g->member[i].sd)!=NULL)
+		struct map_session_data *sd = g->member[i].sd;
+		if(sd != NULL)
 			clif_guild_notice(sd,g);
 	}
 	return 0;
@@ -1660,13 +1661,14 @@ int guild_allianceack(int guild_id1,int guild_id2,int account_id1,int account_id
 
 
 	for (i = 0; i < 2 - (flag & 1); i++) { // Retransmission of the relationship list to all members
-		struct map_session_data *sd;
 		if(g[i]!=NULL)
-			for(j=0;j<g[i]->max_member;j++)
-				if((sd=g[i]->member[j].sd)!=NULL){
+			for(j=0;j<g[i]->max_member;j++) {
+				struct map_session_data *sd = g[i]->member[j].sd;
+				if( sd!=NULL){
 					clif_guild_allianceinfo(sd);
 					channel_gjoin(sd,2); //join ally join
 				}
+			}
 	}
 	return 0;
 }
@@ -1724,14 +1726,14 @@ int castle_guild_broken_sub(DBKey key, DBData *data, va_list ap)
 int guild_broken(int guild_id,int flag)
 {
 	struct guild *g = guild_search(guild_id);
-	struct map_session_data *sd = NULL;
 	int i;
 
 	if(flag!=0 || g==NULL)
 		return 0;
 
 	for(i=0;i<g->max_member;i++){	// Destroy all relationships
-		if((sd=g->member[i].sd)!=NULL){
+		struct map_session_data *sd = sd=g->member[i].sd;
+		if(sd != NULL){
 			if(sd->state.storage_flag == 2)
 				storage_guild_storage_quit(sd,1);
 			sd->status.guild_id=0;
@@ -1873,12 +1875,12 @@ int guild_break(struct map_session_data *sd,char *name)
  */
 void guild_castle_map_init(void)
 {
-	DBIterator* iter = NULL;
 	int num = db_size(castle_db);
 
 	if (num > 0) {
 		struct guild_castle* gc = NULL;
 		int *castle_ids, *cursor;
+		DBIterator* iter = NULL;
 
 		CREATE(castle_ids, int, num);
 		cursor = castle_ids;
@@ -1914,11 +1916,12 @@ int guild_castledatasave(int castle_id, int index, int value)
 	case 1: // The castle's owner has changed? Update or remove Guardians too. [Skotlex]
 	{
 		int i;
-		struct mob_data *gd;
 		gc->guild_id = value;
-		for (i = 0; i < MAX_GUARDIANS; i++)
+		for (i = 0; i < MAX_GUARDIANS; i++){
+			struct mob_data *gd;
 			if (gc->guardian[i].visible && (gd = map_id2md(gc->guardian[i].id)) != NULL)
 				mob_guardian_guildchange(gd);
+		}
 		break;
 	}
 	case 2:
@@ -1926,11 +1929,12 @@ int guild_castledatasave(int castle_id, int index, int value)
 	case 3: // defense invest change -> recalculate guardian hp
 	{
 		int i;
-		struct mob_data *gd;
 		gc->defense = value;
-		for (i = 0; i < MAX_GUARDIANS; i++)
+		for (i = 0; i < MAX_GUARDIANS; i++){
+			struct mob_data *gd;
 			if (gc->guardian[i].visible && (gd = map_id2md(gc->guardian[i].id)) != NULL)
 				status_calc_mob(gd, 0);
+		}
 		break;
 	}
 	case 4:

src/map/intif.c

@@ -938,20 +938,17 @@ int mapif_parse_WisToGM(int fd)
 {
 	int permission, mes_len;
 	char Wisp_name[NAME_LENGTH];
-	char mbuf[255];
 	char *message;
 
 	mes_len =  RFIFOW(fd,2) - 32;
-	message = (char *) (mes_len >= 255 ? (char *) aMalloc(mes_len) : mbuf);
+	message = (char *) aMalloc(mes_len);
 
 	permission = RFIFOL(fd,28);
 	safestrncpy(Wisp_name, (char*)RFIFOP(fd,4), NAME_LENGTH);
 	safestrncpy(message, (char*)RFIFOP(fd,32), mes_len);
 	// information is sent to all online GM
 	map_foreachpc(mapif_parse_WisToGM_sub, permission, Wisp_name, message, mes_len);
-
-	if (message != mbuf)
-		aFree(message);
+	aFree(message);
 	return 0;
 }
 

src/map/map.c

@@ -3380,7 +3380,7 @@ int map_config_read(char *cfgName)
 			continue;
 		if( (ptr = strstr(line, "//")) != NULL )
 			*ptr = '\n'; //Strip comments
-		if( sscanf(line, "%[^:]: %[^\t\r\n]", w1, w2) < 2 )
+		if( sscanf(line, "%1023[^:]: %1023[^\t\r\n]", w1, w2) < 2 )
 			continue;
 
 		//Strip trailing spaces
@@ -3482,7 +3482,7 @@ void map_reloadnpc_sub(char *cfgName)
 			continue;
 		if( (ptr = strstr(line, "//")) != NULL )
 			*ptr = '\n'; //Strip comments
-		if( sscanf(line, "%[^:]: %[^\t\r\n]", w1, w2) < 2 )
+		if( sscanf(line, "%1023[^:]: %1023[^\t\r\n]", w1, w2) < 2 )
 			continue;
 
 		//Strip trailing spaces
@@ -3530,7 +3530,7 @@ int inter_config_read(char *cfgName)
 	{
 		if(line[0] == '/' && line[1] == '/')
 			continue;
-		if( sscanf(line,"%[^:]: %[^\r\n]",w1,w2) < 2 )
+		if( sscanf(line,"%1023[^:]: %1023[^\r\n]",w1,w2) < 2 )
 			continue;
 
 		if(strcmpi(w1,"item_db_db")==0)

src/map/pc.c

@@ -1915,10 +1915,10 @@ static int pc_bonus_item_drop(struct s_add_drop *drop, const short max, short id
 		if(
 			((id && drop[i].id == id) ||
 			(group && drop[i].group == group))
-			&& ((race<RC_NONE_ && race<RC_MAX) || (class_<CLASS_NONE && class_<CLASS_MAX))
+			&& ((race>RC_NONE_ && race<RC_MAX) || (class_>CLASS_NONE && class_<CLASS_MAX))
 		) {
-			if(race<RC_NONE_ && race<RC_MAX) drop[i].race |= 1<<race;
-			if(class_<CLASS_NONE && class_<CLASS_MAX) drop[i].class_ |= 1<<class_;
+			if(race>RC_NONE_ && race<RC_MAX) drop[i].race |= 1<<race;
+			if(class_>CLASS_NONE && class_<CLASS_MAX) drop[i].class_ |= 1<<class_;
 			if(drop[i].rate > 0 && rate > 0)
 			{	//Both are absolute rates.
 				if (drop[i].rate < rate)
@@ -1939,8 +1939,8 @@ static int pc_bonus_item_drop(struct s_add_drop *drop, const short max, short id
 	}
 	drop[i].id = id;
 	drop[i].group = group;
-	if(race<RC_NONE_ && race<RC_MAX) drop[i].race |= 1<<race;
-	if(class_<CLASS_NONE && class_<CLASS_MAX) drop[i].class_ |= 1<<class_;
+	if(race>RC_NONE_ && race<RC_MAX) drop[i].race |= 1<<race;
+	if(class_>CLASS_NONE && class_<CLASS_MAX) drop[i].class_ |= 1<<class_;
 	drop[i].rate = rate;
 	return 1;
 }
@@ -4176,20 +4176,16 @@ int pc_takeitem(struct map_session_data *sd,struct flooritem_data *fitem)
 	if (sd->status.party_id)
 		p = party_search(sd->status.party_id);
 
-	if(fitem->first_get_charid > 0 && fitem->first_get_charid != sd->status.char_id)
-	{
-		struct map_session_data *first_sd = NULL,*second_sd = NULL,*third_sd = NULL;
-		first_sd = map_charid2sd(fitem->first_get_charid);
+	if(fitem->first_get_charid > 0 && fitem->first_get_charid != sd->status.char_id) {
+		struct map_session_data *first_sd = map_charid2sd(fitem->first_get_charid);
 		if(DIFF_TICK(tick,fitem->first_get_tick) < 0) {
 			if (!(p && p->party.item&1 &&
 				first_sd && first_sd->status.party_id == sd->status.party_id
 			))
 				return 0;
 		}
-		else
-		if(fitem->second_get_charid > 0 && fitem->second_get_charid != sd->status.char_id)
-		{
-			second_sd = map_charid2sd(fitem->second_get_charid);
+		else if(fitem->second_get_charid > 0 && fitem->second_get_charid != sd->status.char_id) {
+			struct map_session_data *second_sd = map_charid2sd(fitem->second_get_charid);
 			if(DIFF_TICK(tick, fitem->second_get_tick) < 0) {
 				if(!(p && p->party.item&1 &&
 					((first_sd && first_sd->status.party_id == sd->status.party_id) ||
@@ -4197,10 +4193,8 @@ int pc_takeitem(struct map_session_data *sd,struct flooritem_data *fitem)
 				))
 					return 0;
 			}
-			else
-			if(fitem->third_get_charid > 0 && fitem->third_get_charid != sd->status.char_id)
-			{
-				third_sd = map_charid2sd(fitem->third_get_charid);
+			else if(fitem->third_get_charid > 0 && fitem->third_get_charid != sd->status.char_id){
+				struct map_session_data *third_sd = map_charid2sd(fitem->third_get_charid);
 				if(DIFF_TICK(tick,fitem->third_get_tick) < 0) {
 					if(!(p && p->party.item&1 &&
 						((first_sd && first_sd->status.party_id == sd->status.party_id) ||

src/map/skill.c

@@ -10991,11 +10991,9 @@ int skill_castend_pos2(struct block_list* src, int x, int y, uint16 skill_id, ui
 		break;
 	case GN_CRAZYWEED: {
 			int area = skill_get_splash(GN_CRAZYWEED_ATK, skill_lv);
-			short x1 = 0, y1 = 0;
-
 			for( i = 0; i < 3 + (skill_lv/2); i++ ) {
-				x1 = x - area + rnd()%(area * 2 + 1);
-				y1 = y - area + rnd()%(area * 2 + 1);
+				int x1 = x - area + rnd()%(area * 2 + 1);
+				int y1 = y - area + rnd()%(area * 2 + 1);
 				skill_addtimerskill(src,tick+i*150,0,x1,y1,GN_CRAZYWEED_ATK,skill_lv,-1,0);
 			}
 		}
@@ -13395,11 +13393,10 @@ int skill_check_condition_castbegin(struct map_session_data* sd, uint16 skill_id
 			break;
 		case AS_CLOAKING:
 		{
-			static int dx[] = { 0, 1, 0, -1, -1,  1, 1, -1};
-			static int dy[] = {-1, 0, 1,  0, -1, -1, 1,  1};
-
 			if( skill_lv < 3 && ((sd->bl.type == BL_PC && battle_config.pc_cloak_check_type&1)
 			||	(sd->bl.type != BL_PC && battle_config.monster_cloak_check_type&1) )) { //Check for walls.
+				static int dx[] = { 0, 1, 0, -1, -1,  1, 1, -1};
+				static int dy[] = {-1, 0, 1,  0, -1, -1, 1,  1};
 				int i;
 				ARR_FIND( 0, 8, i, map_getcell(sd->bl.m, sd->bl.x+dx[i], sd->bl.y+dy[i], CELL_CHKNOPASS) != 0 );
 				if( i == 8 ) {
@@ -13552,11 +13549,11 @@ int skill_check_condition_castbegin(struct map_session_data* sd, uint16 skill_id
 			break;
 		case CG_MOONLIT: //Check there's no wall in the range+1 area around the caster. [Skotlex]
 			{
-				int i,x,y,range = skill_get_splash(skill_id, skill_lv)+1;
+				int i,range = skill_get_splash(skill_id, skill_lv)+1;
 				int size = range*2+1;
 				for (i=0;i<size*size;i++) {
-					x = sd->bl.x+(i%size-range);
-					y = sd->bl.y+(i/size-range);
+					int x = sd->bl.x+(i%size-range);
+					int y = sd->bl.y+(i/size-range);
 					if (map_getcell(sd->bl.m,x,y,CELL_CHKWALL)) {
 						clif_skill_fail(sd,skill_id,USESKILL_FAIL_LEVEL,0);
 						return 0;
@@ -13577,11 +13574,11 @@ int skill_check_condition_castbegin(struct map_session_data* sd, uint16 skill_id
 		case HP_BASILICA:
 			if( !sc || (sc && !sc->data[SC_BASILICA])) {
 				if( sd ) {
-					int i,x,y,range = skill_get_unit_range(skill_id,skill_lv)+1;
+					int i,range = skill_get_unit_range(skill_id,skill_lv)+1;
 					int size = range*2+1;
 					for( i=0;i<size*size;i++ ) {
-						x = sd->bl.x+(i%size-range);
-						y = sd->bl.y+(i/size-range);
+						int x = sd->bl.x+(i%size-range);
+						int y = sd->bl.y+(i/size-range);
 						if( map_getcell(sd->bl.m,x,y,CELL_CHKWALL) ) {
 							clif_skill_fail(sd,skill_id,USESKILL_FAIL,0);
 							return 0;
@@ -14170,7 +14167,7 @@ int skill_check_condition_castend(struct map_session_data* sd, uint16 skill_id,
 		}
 		case NC_SILVERSNIPER:
 		case NC_MAGICDECOY: {
-				int c = 0, j;
+				int c = 0;
 				int maxcount = skill_get_maxcount(skill_id,skill_lv);
 				int mob_class = MOBID_SILVERSNIPER;
 				if( skill_id == NC_MAGICDECOY )
@@ -14178,6 +14175,7 @@ int skill_check_condition_castend(struct map_session_data* sd, uint16 skill_id,
 
 				if( battle_config.land_skill_limit && maxcount > 0 && ( battle_config.land_skill_limit&BL_PC ) ) {
 					if( skill_id == NC_MAGICDECOY ) {
+						int j;
 						for( j = mob_class; j <= MOBID_MAGICDECOY_WIND; j++ )
 							map_foreachinmap(skill_check_condition_mob_master_sub, sd->bl.m, BL_MOB, sd->bl.id, j, skill_id, &c);
 					} else
@@ -15241,14 +15239,14 @@ void skill_weaponrefine (struct map_session_data *sd, int idx)
 
 	if (idx >= 0 && idx < MAX_INVENTORY)
 	{
-		int i = 0, ep = 0, per;
-		int material[5] = { 0, ITEMID_PHRACON, ITEMID_EMVERETARCON, ITEMID_ORIDECON, ITEMID_ORIDECON, };
 		struct item *item;
 		struct item_data *ditem = sd->inventory_data[idx];
 		item = &sd->status.inventory[idx];
 
 		if(item->nameid > 0 && ditem->type == IT_WEAPON)
 		{
+			int i = 0, ep = 0, per;
+			int material[5] = { 0, ITEMID_PHRACON, ITEMID_EMVERETARCON, ITEMID_ORIDECON, ITEMID_ORIDECON, };
 			if( ditem->flag.no_refine ) { 	// if the item isn't refinable
 				clif_skill_fail(sd,sd->menuskill_id,USESKILL_FAIL_LEVEL,0);
 				return;
@@ -15951,13 +15949,13 @@ int skill_enchant_elemental_end (struct block_list *bl, int type)
 
 bool skill_check_cloaking(struct block_list *bl, struct status_change_entry *sce)
 {
-	static int dx[] = { 0, 1, 0, -1, -1,  1, 1, -1};
-	static int dy[] = {-1, 0, 1,  0, -1, -1, 1,  1};
 	bool wall = true;
 
 	if( (bl->type == BL_PC && battle_config.pc_cloak_check_type&1)
 	||	(bl->type != BL_PC && battle_config.monster_cloak_check_type&1) )
 	{	//Check for walls.
+		static int dx[] = { 0, 1, 0, -1, -1,  1, 1, -1};
+		static int dy[] = {-1, 0, 1,  0, -1, -1, 1,  1};
 		int i;
 		ARR_FIND( 0, 8, i, map_getcell(bl->m, bl->x+dx[i], bl->y+dy[i], CELL_CHKNOPASS) != 0 );
 		if( i == 8 )
@@ -15984,11 +15982,11 @@ bool skill_check_cloaking(struct block_list *bl, struct status_change_entry *sce
 }
 bool skill_check_camouflage(struct block_list *bl, struct status_change_entry *sce)
 {
-	static int dx[] = { 0, 1, 0, -1, -1,  1, 1, -1};
-	static int dy[] = {-1, 0, 1,  0, -1, -1, 1,  1};
 	bool wall = true;
 
 	if( bl->type == BL_PC ) { //Check for walls.
+		static int dx[] = { 0, 1, 0, -1, -1,  1, 1, -1};
+		static int dy[] = {-1, 0, 1,  0, -1, -1, 1,  1};
 		int i;
 		ARR_FIND( 0, 8, i, map_getcell(bl->m, bl->x+dx[i], bl->y+dy[i], CELL_CHKNOPASS) != 0 );
 		if( i == 8 )
@@ -16188,12 +16186,14 @@ struct skill_unit_group* skill_initunitgroup (struct block_list* src, int count,
 	if(i == MAX_SKILLUNITGROUP) {
 		// array is full, make room by discarding oldest group
 		int j=0;
-		unsigned maxdiff=0,x,tick=gettick();
-		for(i=0;i<MAX_SKILLUNITGROUP && ud->skillunit[i];i++)
-			if((x=DIFF_TICK(tick,ud->skillunit[i]->tick))>maxdiff){
+		unsigned maxdiff=0,tick=gettick();
+		for(i=0;i<MAX_SKILLUNITGROUP && ud->skillunit[i];i++){
+			unsigned int x=DIFF_TICK(tick,ud->skillunit[i]->tick);
+			if(x>maxdiff){
 				maxdiff=x;
 				j=i;
 			}
+		}
 		skill_delunitgroup(ud->skillunit[j]);
 		//Since elements must have shifted, we use the last slot.
 		i = MAX_SKILLUNITGROUP-1;
@@ -16384,7 +16384,7 @@ int skill_clear_unitgroup (struct block_list *src)
  *
  *------------------------------------------*/
 struct skill_unit_group_tickset *skill_unitgrouptickset_search (struct block_list *bl, struct skill_unit_group *group, int tick) {
-	int i,j=-1,k,s,id;
+	int i,j=-1,s,id;
 	struct unit_data *ud;
 	struct skill_unit_group_tickset *set;
 
@@ -16403,7 +16403,7 @@ struct skill_unit_group_tickset *skill_unitgrouptickset_search (struct block_lis
 		id = s = group->group_id;
 
 	for (i=0; i<MAX_SKILLUNITGROUPTICKSET; i++) {
-		k = (i+s) % MAX_SKILLUNITGROUPTICKSET;
+		int k = (i+s) % MAX_SKILLUNITGROUPTICKSET;
 		if (set[k].id == id)
 			return &set[k];
 		else if (j==-1 && (DIFF_TICK(tick,set[k].tick)>0 || set[k].id==0))
@@ -16941,7 +16941,7 @@ int skill_can_produce_mix (struct map_session_data *sd, int nameid, int trigger,
 	}
 
 	for(j=0;j<MAX_PRODUCE_RESOURCE;j++){
-		int id,x,y;
+		int id;
 		if( (id=skill_produce_db[i].mat_id[j]) <= 0 )
 			continue;
 		if(skill_produce_db[i].mat_amount[j] <= 0) {
@@ -16949,6 +16949,7 @@ int skill_can_produce_mix (struct map_session_data *sd, int nameid, int trigger,
 				return 0;
 		}
 		else {
+			int x,y;
 			for(y=0,x=0;y<MAX_INVENTORY;y++)
 				if( sd->status.inventory[y].nameid == id )
 					x+=sd->status.inventory[y].amount;
@@ -17767,7 +17768,7 @@ int skill_elementalanalysis(struct map_session_data* sd, int n, uint16 skill_lv,
 		return 1;
 
 	for( i = 0; i < n; i++ ) {
-		int nameid, add_amount, del_amount, idx, product, flag;
+		int nameid, add_amount, del_amount, idx, product;
 		struct item tmp_item;
 
 		idx = item_list[i*2+0]-2;
@@ -17815,7 +17816,8 @@ int skill_elementalanalysis(struct map_session_data* sd, int n, uint16 skill_lv,
 		tmp_item.identify = 1;
 
 		if( tmp_item.amount ) {
-			if( (flag = pc_additem(sd,&tmp_item,tmp_item.amount,LOG_TYPE_CONSUME)) ) {
+			int flag = pc_additem(sd,&tmp_item,tmp_item.amount,LOG_TYPE_CONSUME);
+			if( flag != 0 ) {
 				clif_additem(sd,0,0,flag);
 				map_addflooritem(&tmp_item,tmp_item.amount,sd->bl.m,sd->bl.x,sd->bl.y,0,0,0,0);
 			}
@@ -18079,11 +18081,16 @@ int skill_split_str (char *str, char **val, int num) {
 
 	return i;
 }
-/*
- *
+
+/**
+ * Split the string with ':' as separator and put each value for a skilllv
+ * if no more value found put the latest to fill the array
+ * @param str : string to split
+ * @param val : array of MAX_SKILL_LEVEL to put value into
+ * @return 0:error, x:number of value assign (should be MAX_SKILL_LEVEL)
  */
 int skill_split_atoi (char *str, int *val) {
-	int i, j, diff, step = 1;
+	int i, j, step = 1;
 
 	for (i=0; i<MAX_SKILL_LEVEL; i++) {
 		if (!str) break;
@@ -18101,7 +18108,7 @@ int skill_split_atoi (char *str, int *val) {
 	}
 	//Check for linear change with increasing steps until we reach half of the data acquired.
 	for (step = 1; step <= i/2; step++) {
-		diff = val[i-1] - val[i-step-1];
+		int diff = val[i-1] - val[i-step-1];
 		for(j = i-1; j >= step; j--)
 			if ((val[j]-val[j-step]) != diff)
 				break;
@@ -18126,13 +18133,13 @@ int skill_split_atoi (char *str, int *val) {
  *
  */
 void skill_init_unit_layout (void) {
-	int i,j,size,pos = 0;
+	int i,j,pos = 0;
 
 	memset(skill_unit_layout,0,sizeof(skill_unit_layout));
 
 	// standard square layouts go first
 	for (i=0; i<=MAX_SQUARE_LAYOUT; i++) {
-		size = i*2+1;
+		int size = i*2+1;
 		skill_unit_layout[i].count = size*size;
 		for (j=0; j<size*size; j++) {
 			skill_unit_layout[i].dx[j] = (j%size-i);