git-svn-id: https://svn.code.sf.net/p/rathena/svn/branches/stable@1213 54d463be-8e91-2dee-dedb-b68131a5f0ec

Changelog-SVN.txt

@@ -1,6 +1,11 @@
 Date	Added
 
 03/08
+        * Reverted the reverted jA event change but this time without bugs (hopefully) [Shinomori]
+        * corrected status_change_timer as far as I understand the functionality
+          (better have a look at it, Celest), added a some security to prevent pending timers
+        * moved two variable declarations to scope start (mob.c and skill.c)
+
         * Fixes Icewall can be directly cast on players and monsters -- also removes
           the 'unsupported layout' message [celest]
         * Added the new turbo_room and alde_tt to the maps config [celest]
@@ -40,6 +45,8 @@ Date	Added
 	  It also auto-ban hackers and broadcasts messages to all GMs. Good work, Yor!
 	* Misc fixes. [Lupus]
 	  2Shino: BTW Some players can't re-connect to the server. Due to the updated session checks in chrif.c
+         actually impossible, because the checks handle the connection with the char server, not with users
+         anyway, I checked again and rearranged code but functionality is still the same and valid [Shinomori]
 03/05
 	* Reversed drop_rate0item option, corrected MVP Drop rate (thanks to Freya) [Lupus]
 03/04

src/map/chrif.c

@@ -100,7 +100,7 @@ int chrif_save(struct map_session_data *sd)
 {
 	nullpo_retr(-1, sd);
 
-	if (char_fd < 1 || session[char_fd] == NULL || !chrif_isconnect())
+	if( char_fd < 1 || session[char_fd] == NULL || !chrif_isconnect() )
 		return -1;
 
 	pc_makesavestatus(sd);
@@ -189,7 +189,7 @@ int chrif_changemapserver(struct map_session_data *sd, char *name, int x, int y,
 
 	nullpo_retr(-1, sd);
 
-	if (!sd || char_fd < 1 || session[char_fd] == NULL || !chrif_isconnect() )
+	if( !sd || char_fd < 1 || session[char_fd] == NULL || !chrif_isconnect() )
 		return -1;
 
 	s_ip = 0;
@@ -299,7 +299,9 @@ int chrif_authreq(struct map_session_data *sd)
 
 	nullpo_retr(-1, sd);
 
-	if (!sd || char_fd < 1 || session[char_fd] == NULL || !chrif_isconnect() || !sd->bl.id || !sd->login_id1)
+	if(!sd || !sd->bl.id || !sd->login_id1)
+		return -1;
+	if( char_fd < 1 || session[char_fd] == NULL || !chrif_isconnect() )
 		return -1;
 
 	for(i = 0; i < fd_max; i++)
@@ -327,7 +329,9 @@ int chrif_charselectreq(struct map_session_data *sd)
 
 	nullpo_retr(-1, sd);
 
-	if(!sd || char_fd < 1 || session[char_fd] == NULL || !chrif_isconnect() || !sd->bl.id || !sd->login_id1)
+	if( !sd || !sd->bl.id || !sd->login_id1 )
+		return -1;
+	if( char_fd < 1 || session[char_fd] == NULL || !chrif_isconnect() )
 		return -1;
 
 	s_ip = 0;
@@ -353,7 +357,9 @@ int chrif_charselectreq(struct map_session_data *sd)
  */
 int chrif_searchcharid(int char_id)
 {
-	if (!char_id || char_fd < 1 || session[char_fd] == NULL || !chrif_isconnect())
+	if( !char_id )
+		return -1;
+	if( char_fd < 1 || session[char_fd] == NULL || !chrif_isconnect() )
 		return -1;
 
 	WFIFOW(char_fd,0) = 0x2b08;
@@ -372,7 +378,7 @@ int chrif_changegm(int id, const char *pass, int len)
 	if (battle_config.etc_log)
 		printf("chrif_changegm: account: %d, password: '%s'.\n", id, pass);
 
-	if (char_fd < 1 || session[char_fd] == NULL || !chrif_isconnect() )
+	if( char_fd < 1 || session[char_fd] == NULL || !chrif_isconnect() )
 		return -1;
 
 	WFIFOW(char_fd,0) = 0x2b0a;
@@ -393,7 +399,7 @@ int chrif_changeemail(int id, const char *actual_email, const char *new_email)
 	if (battle_config.etc_log)
 		printf("chrif_changeemail: account: %d, actual_email: '%s', new_email: '%s'.\n", id, actual_email, new_email);
 
-	if (char_fd < 1 || session[char_fd] == NULL || !chrif_isconnect() )
+	if( char_fd < 1 || session[char_fd] == NULL || !chrif_isconnect() )
 		return -1;
 
 	WFIFOW(char_fd,0) = 0x2b0c;
@@ -418,7 +424,7 @@ int chrif_changeemail(int id, const char *actual_email, const char *new_email)
  */
 int chrif_char_ask_name(int id, char * character_name, short operation_type, int year, int month, int day, int hour, int minute, int second)
 {
-   	if (char_fd < 1 || session[char_fd] == NULL || !chrif_isconnect() )
+	if( char_fd < 1 || session[char_fd] == NULL || !chrif_isconnect() )
 		return -1;
 
 	WFIFOW(char_fd, 0) = 0x2b0e;
@@ -444,7 +450,7 @@ int chrif_char_ask_name(int id, char * character_name, short operation_type, int
  *------------------------------------------
  */
 int chrif_changesex(int id, int sex) {
-   	if (char_fd < 1 || session[char_fd] == NULL || !chrif_isconnect() )
+	if( char_fd < 1 || session[char_fd] == NULL || !chrif_isconnect() )
 		return -1;
 
 	WFIFOW(char_fd,0) = 0x2b11;
@@ -678,7 +684,7 @@ int chrif_saveaccountreg2(struct map_session_data *sd)
 	int p, j;
 	nullpo_retr(-1, sd);
 
-	if (char_fd < 1 || session[char_fd] == NULL || !chrif_isconnect() )
+	if( char_fd < 1 || session[char_fd] == NULL || !chrif_isconnect() )
 		return -1;
 
 	p = 8;
@@ -851,7 +857,7 @@ int chrif_chardisconnect(struct map_session_data *sd)
 {
 	nullpo_retr(-1, sd);
 
-	if(char_fd < 1 || session[char_fd] == NULL || !chrif_isconnect())
+	if( char_fd < 1 || session[char_fd] == NULL || !chrif_isconnect() )
 		return -1;
 
 	WFIFOW(char_fd,0)=0x2b18;
@@ -881,7 +887,7 @@ int chrif_recvgmaccounts(int fd)
  */
 int chrif_reloadGMdb(void)
 {
-   	if (char_fd < 1 || session[char_fd] == NULL || !chrif_isconnect() )
+	if( char_fd < 1 || session[char_fd] == NULL || !chrif_isconnect() )
 		return -1;
 
 	WFIFOW(char_fd,0) = 0x2af7;
@@ -900,7 +906,7 @@ int chrif_reloadGMdb(void)
 	FILE *fp;
 	int i;
 
-	if (char_fd < 1 || session[char_fd] == NULL || !chrif_isconnect() )
+	if( char_fd < 1 || session[char_fd] == NULL || !chrif_isconnect() )
 		return -1;
 
 	WFIFOW(char_fd,0) = 0x2b16;
@@ -936,7 +942,7 @@ int chrif_reloadGMdb(void)
 
 int chrif_char_offline(struct map_session_data *sd)
 {
-	if (char_fd < 1 || session[char_fd] == NULL || !chrif_isconnect())
+	if( char_fd < 1 || session[char_fd] == NULL || !chrif_isconnect() )
 		return -1;
 
 	WFIFOW(char_fd,0) = 0x2b17;
@@ -952,7 +958,7 @@ int chrif_char_offline(struct map_session_data *sd)
  *-----------------------------------------
  */
 int chrif_flush_fifo(void) {
-	if (char_fd < 1 || session[char_fd] == NULL || !chrif_isconnect())
+	if( char_fd < 1 || session[char_fd] == NULL || !chrif_isconnect() )
 		return -1;
 
 	set_nonblocking(char_fd, 0);
@@ -967,7 +973,7 @@ int chrif_flush_fifo(void) {
  *-----------------------------------------
  */
 int chrif_char_reset_offline(void) {
-	if (char_fd < 1 || session[char_fd] == NULL || !chrif_isconnect())
+	if( char_fd < 1 || session[char_fd] == NULL || !chrif_isconnect() )
 		return -1;
 
 	WFIFOW(char_fd,0) = 0x2b18;
@@ -983,7 +989,7 @@ int chrif_char_reset_offline(void) {
 
 int chrif_char_online(struct map_session_data *sd)
 {
-	if (char_fd < 1 || session[char_fd] == NULL || !chrif_isconnect())
+	if( char_fd < 1 || session[char_fd] == NULL || !chrif_isconnect() )
 		return -1;
 
 	WFIFOW(char_fd,0) = 0x2b19;
@@ -1099,7 +1105,7 @@ int send_users_tochar(int tid, unsigned int tick, int id, int data) {
 	int users = 0, i;
 	struct map_session_data *sd;
 
-	if (char_fd < 1 || session[char_fd] == NULL || !chrif_isconnect()) // Thanks to Toster
+	if( char_fd < 1 || session[char_fd] == NULL || !chrif_isconnect() ) // Thanks to Toster
 		return 0;
 
 	WFIFOW(char_fd,0) = 0x2aff;

src/map/mob.c

@@ -135,9 +135,9 @@ int mob_once_spawn(struct map_session_data *sd,char *mapname,
 		return 0;
 
 	if(class_<0){	// ランダムに召喚
+		int k;
 		i = 0;
 		j = -class_-1;
-		int k;
 		if(j>=0 && j<MAX_RANDOMMONSTER){
 			do{
 				class_=rand()%1000+1001;

src/map/npc.c

@@ -1664,7 +1664,15 @@ int npc_convertlabel_db(void *key,void *data,va_list ap)
 		lst=(struct npc_label_list *)aRealloc(lst,sizeof(struct npc_label_list)*(num+1));
 
 	*p='\0';
-	strncpy(lst[num].name,lname,24);
+	
+	// here we check if the label fit into the buffer
+	if (strlen(lname)>23) { 
+		printf("npc_parse_script: label name longer than 23 chars! '%s'\n", lname);
+		exit(1);
+	}
+	memcpy(lst[num].name,lname,strlen(lname)+1); //including EOS
+	
+	
 	*p=':';
 	lst[num].pos=pos;
 	nd->u.scr.label_list=lst;
@@ -1873,11 +1881,16 @@ static int npc_parse_script(char *w1,char *w2,char *w3,char *w4,char *first_line
 		int pos=nd->u.scr.label_list[i].pos;
 
 		if ((lname[0]=='O' || lname[0]=='o')&&(lname[1]=='N' || lname[1]=='n')) {
+/*
+I rearrange the code so this is just for commenting; remove it if you have enough if it [Shinomori]
 			struct event_data *ev;
 			char *buf;
 			// ƒGƒNƒXƒ|<7C>[ƒg‚³‚ê‚é
 			ev=(struct event_data *)aCalloc(1,sizeof(struct event_data));
+why allocing 50 chars ?
 			buf=(char *)aCallocA(50,sizeof(char));
+why checking here? 
+lname is identical to nd->u.scr.label_list[i].name which is only 24 chars so check for strlen should be 23
 			if (strlen(lname)>24) {
 				printf("npc_parse_script: label name error !\n");
 				exit(1);
@@ -1891,7 +1904,47 @@ static int npc_parse_script(char *w1,char *w2,char *w3,char *w4,char *first_line
 				//	printf("npc_parse_script : duplicate event %s\n",buf);
 				//	aFree(ev2);
 				//}
+you are sure reentering the same database key will overwrite the existing entry?
 				strdb_insert(ev_db,buf,ev);
+anyway instead of removing data from the db and inserting a new one
+wouldn't it be easier just not to insert the new duplicate event, it is a duplicate anyway?
+			}	
+*/
+			// this check is useless here because the buffer is only 24 chars 
+			// and already overwritten if this is here is reached
+			// I leave the check anyway but place it correctly to npc_convertlabel_db
+			if (strlen(lname)>23) { 
+				printf("npc_parse_script: label name longer than 23 chars! '%s'\n", lname);
+				exit(1);
+			}else{
+				struct event_data *ev;
+				struct event_data *ev2;
+				char *buf;
+				// ƒGƒNƒXƒ|<7C>[ƒg‚³‚ê‚é
+				
+				// 51 comes from: 24 for npc name + 24 for label + 2 for a "::" and 1 for EOS
+				//buf=(char *)aMalloc(51,sizeof(char)); 
+				// but to save some memory we alloc only the really necessary space
+				buf=(char *)aMalloc( (3+strlen(nd->exname)+strlen(lname))*sizeof(char));
+				sprintf(buf,"%s::%s",nd->exname,lname);
+				
+				// search the label in ev_db; 
+				// remember the label is max 50 chars + eos; see the strdb_init below
+				ev2 = (struct event_data *)strdb_search(ev_db,buf);
+				if(ev2 != NULL) {
+					printf("npc_parse_script : duplicate event %s\n",buf);
+					
+					// just skip the label insertion and free the alloced buffer
+					aFree(buf);
+				}
+				else
+				{	// generate the data and insert it
+					ev=(struct event_data *)aCalloc(1,sizeof(struct event_data));
+					ev->nd=nd;
+					ev->pos=pos;
+					strdb_insert(ev_db,buf,ev);
+				}
+
 			}
 		}
 	}
@@ -2362,7 +2415,10 @@ int do_init_npc(void)
 	npc_read_indoors();
 	//npc_read_weather();
 
-	ev_db=strdb_init(24);
+	// comparing only the first 24 chars of labels that are 50 chars long isn't that nice
+	// will cause "duplicated" labels where actually no dup is...
+	//ev_db=strdb_init(24); 
+	ev_db=strdb_init(51);
 	npcname_db=strdb_init(24);
 
         ev_db->release = ev_release;

src/map/skill.c

@@ -6979,6 +6979,7 @@ int skill_use_id( struct map_session_data *sd, int target_id,
 		break;
 	case HP_BASILICA:		/* ƒoƒWƒŠƒJ */
 		{
+			struct status_change *sc_data;
 			if (skill_check_unit_range(sd->bl.m,sd->bl.x,sd->bl.y,sd->skillid,sd->skilllv)) {
 				clif_skill_fail(sd,sd->skillid,0,0);
 				return 0;
@@ -6988,7 +6989,7 @@ int skill_use_id( struct map_session_data *sd, int target_id,
 				return 0;
 			}
 			// cancel Basilica if already in effect
-			struct status_change *sc_data = status_get_sc_data(&sd->bl);
+			sc_data = status_get_sc_data(&sd->bl);
 			if(sc_data && sc_data[SC_BASILICA].timer != -1) {
 				struct skill_unit_group *sg = (struct skill_unit_group *)sc_data[SC_BASILICA].val4;
 				if (sg && sg->src_id == sd->bl.id) {

src/map/status.c

@@ -4274,6 +4274,9 @@ int status_change_timer(int tid, unsigned int tick, int id, int data)
 	struct status_change *sc_data;
 	//short *sc_count; //使ってない？
 
+// security system to prevent forgetting timer removal
+	int temp_timerid;
+
 	nullpo_retr_f(0, bl=map_id2bl(id), "id=%d data=%d",id,data);
 	nullpo_retr(0, sc_data=status_get_sc_data(bl));
 
@@ -4288,6 +4291,11 @@ int status_change_timer(int tid, unsigned int tick, int id, int data)
 		return 0;
 	}
 
+	// security system to prevent forgetting timer removal
+	// you shouldn't be that careless inside the switch here
+	temp_timerid = sc_data[type].timer;
+	sc_data[type].timer = -1;
+
 	switch(type){	/* 特殊な?理になる場合 */
 	case SC_MAXIMIZEPOWER:	/* マキシマイズパワ? */
 	case SC_CLOAKING:
@@ -4417,9 +4425,11 @@ int status_change_timer(int tid, unsigned int tick, int id, int data)
 				break;
 			skill_additional_effect(bl,bl,unit->group->skill_id,sc_data[type].val1,BF_LONG|BF_SKILL|BF_MISC,tick);
 			if (unit->group != 0)
+			{
 				sc_data[type].timer=add_timer(skill_get_time(unit->group->skill_id,unit->group->skill_lv)/10+tick,
 					status_change_timer, bl->id, data );
-			return 0;
+				return 0;
+			}// dont forget the brackets
 		}
 		break;
 
@@ -4473,10 +4483,19 @@ int status_change_timer(int tid, unsigned int tick, int id, int data)
 					}
 				}
 				sc_data[type].timer=add_timer(1000+tick,status_change_timer, bl->id, data );
+				// hmm setting up a timer and breaking then to call status_change_end just right away?
+				// I think you're missing a:
+				return 0;
+				
 			}
 		}
 		else
+		{
 			sc_data[type].timer=add_timer(1000+tick,status_change_timer, bl->id, data );
+			// hmm setting up a timer and breaking then to call status_change_end just right away?
+			// I think you're missing brackets and a:
+			return 0;
+		}
 		break;
 	case SC_DPOISON:
 		if (sc_data[SC_SLOWPOISON].timer == -1 && (--sc_data[type].val3) > 0) {
@@ -4496,7 +4515,12 @@ int status_change_timer(int tid, unsigned int tick, int id, int data)
 			}
 		}
 		if (sc_data[type].val3 > 0)
+		{
 			sc_data[type].timer=add_timer(1000+tick,status_change_timer, bl->id, data );
+			// hmm setting up a timer and breaking then to call status_change_end just right away?
+			// I think you're missing brackets and a:
+			return 0;
+		}
 		break;
 
 	case SC_TENSIONRELAX:	/* テンションリラックス */
@@ -4512,7 +4536,13 @@ int status_change_timer(int tid, unsigned int tick, int id, int data)
 				return 0;
 			}
 			if(sd->status.max_hp <= sd->status.hp)
+			{
 				status_change_end(&sd->bl,SC_TENSIONRELAX,-1);
+				// calling status_change_end then break and call it again might not be that what is necessary
+				// or am I wrong?
+				// so I just add brackets and a:
+				return 0;
+			}
 		}
 		break;
 	case SC_BLEEDING:	// [celest]
@@ -4538,6 +4568,9 @@ int status_change_timer(int tid, unsigned int tick, int id, int data)
 			}
 
 			sc_data[type].timer=add_timer(1000+tick,status_change_timer, bl->id, data );
+			// hmm setting up a timer and breaking then to call status_change_end just right away?
+			// I think you're missing a:
+			return 0;
 		}
 		break;
 
@@ -4554,8 +4587,7 @@ int status_change_timer(int tid, unsigned int tick, int id, int data)
 	case SC_BROKNWEAPON:
 	case SC_BROKNARMOR:
 	case SC_SACRIFICE:
-//		if(sc_data[type].timer==tid)
+		sc_data[type].timer=add_timer( 1000*600+tick,status_change_timer, bl->id, data );
-			sc_data[type].timer=add_timer( 1000*600+tick,status_change_timer, bl->id, data );
 		return 0;
 
 	case SC_DANCING: //ダンススキルの時間SP消費
@@ -4862,15 +4894,24 @@ int status_change_timer(int tid, unsigned int tick, int id, int data)
 	case SC_GUILDAURA:
 		{
 			struct block_list *tbl = map_id2bl(sc_data[type].val2);
-			if (tbl && battle_check_range(bl, tbl, 2))
+			
+			if (tbl && battle_check_range(bl, tbl, 2)){
 				sc_data[type].timer = add_timer(
 					1000 + tick, status_change_timer,
 					bl->id, data);
 					return 0;
+			}// ugh, don't  forget the brackets
 		}
 		break;
 	}
 	
+	// default for all non-handled control paths
+	// security system to prevent forgetting timer removal
+
+	// if we reach this point we need the timer for the next call, 
+	// so restore it to have status_change_end handle a valid timer
+	sc_data[type].timer = temp_timerid; 
+
 	return status_change_end( bl,type,tid );
 }