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
rathena · Jan 9, 2014 · b92230e · b92230e
1 parent 4af3145
commit b92230e
Show file tree

Hide file tree

Showing 11 changed files with 99 additions and 107 deletions.
diff --git a/src/common/socket.c b/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")) {

diff --git a/src/common/timer.c b/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 {

diff --git a/src/login/login.c b/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;

diff --git a/src/map/battle.c b/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;

diff --git a/src/map/channel.c b/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]);
 		}

diff --git a/src/map/clif.c b/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;

diff --git a/src/map/guild.c b/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,23 +1916,25 @@ 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:
 		gc->economy = value; break;
 	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:

diff --git a/src/map/intif.c b/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;
 }