From bd2109d614a443045c7bbbf632bb9035216e3623 Mon Sep 17 00:00:00 2001 From: Haru Date: Tue, 19 May 2015 01:59:33 +0200 Subject: Fixed some issues reported by coverity scan [3/3] - Automatically zeroed variables are now zeroed in the correct size, regardless of padding. - Special thanks to Ind. Signed-off-by: Haru --- src/common/cbasetypes.h | 6 ++++ src/map/map.c | 5 +-- src/map/map.h | 4 +-- src/map/pc.c | 14 +++----- src/map/pc.h | 28 ++++++++-------- src/map/skill.c | 27 +++------------ src/map/skill.h | 2 ++ src/map/status.c | 88 ++++--------------------------------------------- src/map/status.h | 5 ++- 9 files changed, 44 insertions(+), 135 deletions(-) diff --git a/src/common/cbasetypes.h b/src/common/cbasetypes.h index 3ff0db795..dda9c8c42 100644 --- a/src/common/cbasetypes.h +++ b/src/common/cbasetypes.h @@ -419,4 +419,10 @@ typedef char bool; #define h64BPTRSIZE(y) (y) #endif +/** Support macros for marking blocks to memset to 0 */ +#define BEGIN_ZEROED_BLOCK int8 HERC__zeroed_block_BEGIN +#define END_ZEROED_BLOCK int8 HERC__zeroed_block_END +#define ZEROED_BLOCK_POS(x) (&(x)->HERC__zeroed_block_BEGIN) +#define ZEROED_BLOCK_SIZE(x) ((void*)&((x)->HERC__zeroed_block_END) - (void*)&((x)->HERC__zeroed_block_BEGIN) + sizeof((x)->HERC__zeroed_block_END)) + #endif /* COMMON_CBASETYPES_H */ diff --git a/src/map/map.c b/src/map/map.c index c79d49c3e..a407722bb 100644 --- a/src/map/map.c +++ b/src/map/map.c @@ -6140,10 +6140,7 @@ void map_defaults(void) { map->bl_list_size = 0; //all in a big chunk, respects order - memset(&map->bl_head,0,sizeof(map->bl_head) - + sizeof(map->zone_all) - + sizeof(map->zone_pk) - ); + memset(ZEROED_BLOCK_POS(map), 0, ZEROED_BLOCK_SIZE(map)); map->cpsd = NULL; map->list = NULL; diff --git a/src/map/map.h b/src/map/map.h index 3960a64b4..02e93b7bb 100644 --- a/src/map/map.h +++ b/src/map/map.h @@ -896,15 +896,15 @@ struct map_interface { DBMap* regen_db; // int id -> struct block_list* (status_natural_heal processing) DBMap* zone_db; // string => struct map_zone_data DBMap* iwall_db; - /* order respected by map_defaults() in order to zero */ - /* from block_free until zone_pk */ struct block_list **block_free; int block_free_count, block_free_lock, block_free_list_size; struct block_list **bl_list; int bl_list_count, bl_list_size; +BEGIN_ZEROED_BLOCK; // This block is zeroed in map_defaults() struct block_list bl_head; struct map_zone_data zone_all;/* used as a base on all maps */ struct map_zone_data zone_pk;/* used for (pk_mode) */ +END_ZEROED_BLOCK; /* */ struct map_session_data *cpsd; struct map_data *list; diff --git a/src/map/pc.c b/src/map/pc.c index 7ae446c90..5fc6469f3 100644 --- a/src/map/pc.c +++ b/src/map/pc.c @@ -11047,16 +11047,10 @@ void pc_defaults(void) { /* */ pc->day_timer_tid = INVALID_TIMER; pc->night_timer_tid = INVALID_TIMER; - /* respecting order */ - memset(pc->exp_table, 0, sizeof(pc->exp_table) - + sizeof(pc->max_level) - + sizeof(pc->statp) - + sizeof(pc->level_penalty) - + sizeof(pc->skill_tree) - + sizeof(pc->smith_fame_list) - + sizeof(pc->chemist_fame_list) - + sizeof(pc->taekwon_fame_list) - ); + + // These macros are used instead of a sum of sizeof(), to ensure that padding won't interfere with our size, and code won't rot when adding more fields + memset(ZEROED_BLOCK_POS(pc), 0, ZEROED_BLOCK_SIZE(pc)); + /* */ memcpy(pc->equip_pos, &equip_pos, sizeof(pc->equip_pos)); /* */ diff --git a/src/map/pc.h b/src/map/pc.h index 867344d58..c3bf1e4cc 100644 --- a/src/map/pc.h +++ b/src/map/pc.h @@ -59,9 +59,7 @@ enum equip_index { }; struct weapon_data { int atkmods[3]; - // all the variables except atkmods get zero'ed in each call of status_calc_pc - // NOTE: if you want to add a non-zeroed variable, you need to update the memset call - // in status_calc_pc as well! All the following are automatically zero'ed. [Skotlex] +BEGIN_ZEROED_BLOCK; // all the variables within this block get zero'ed in each call of status_calc_pc int overrefine; int star; int ignore_def_ele; @@ -85,6 +83,7 @@ struct weapon_data { short flag, rate; unsigned char ele; } addele2[MAX_PC_BONUS]; +END_ZEROED_BLOCK; }; struct s_autospell { short id, lv, rate, card_id, flag; @@ -264,7 +263,8 @@ struct map_session_data { short weapontype1,weapontype2; short disguise; // [Valaris] struct weapon_data right_weapon, left_weapon; - // here start arrays to be globally zeroed at the beginning of status_calc_pc() + +BEGIN_ZEROED_BLOCK; // this block will be globally zeroed at the beginning of status_calc_pc() int param_bonus[6],param_equip[6]; //Stores card/equipment bonuses. int subele[ELE_MAX]; int subrace[RC_MAX]; @@ -292,8 +292,6 @@ struct map_session_data { #ifdef RENEWAL int race_tolerance[RC_MAX]; #endif - // zeroed arrays end here. - // zeroed structures start here struct s_autospell autospell[15], autospell2[15], autospell3[15]; struct s_addeffect addeff[MAX_PC_BONUS], addeff2[MAX_PC_BONUS]; struct s_addeffectonskill addeff3[MAX_PC_BONUS]; @@ -322,11 +320,6 @@ struct map_session_data { short value; int rate, tick; } def_set_race[RC_MAX], mdef_set_race[RC_MAX]; - // zeroed structures end here - // manually zeroed structures start here. - struct s_autobonus autobonus[MAX_PC_BONUS], autobonus2[MAX_PC_BONUS], autobonus3[MAX_PC_BONUS]; //Auto script on attack, when attacked, on skill usage - // manually zeroed structures end here. - // zeroed vars start here. struct { int atk_rate; int arrow_atk,arrow_ele,arrow_cri,arrow_hit; @@ -363,7 +356,11 @@ struct map_session_data { int add_fixcast,add_varcast; int ematk; // matk bonus from equipment } bonus; - // zeroed vars end here. +END_ZEROED_BLOCK; + + // The following structures are zeroed manually in status_calc_pc_ + struct s_autobonus autobonus[MAX_PC_BONUS], autobonus2[MAX_PC_BONUS], autobonus3[MAX_PC_BONUS]; //Auto script on attack, when attacked, on skill usage + int castrate,delayrate,hprate,sprate,dsprate; int hprecov_rate,sprecov_rate; int matk_rate; @@ -753,17 +750,20 @@ struct pc_interface { int day_timer_tid; int night_timer_tid; /* */ + +BEGIN_ZEROED_BLOCK; /* Everything within this block will be memset to 0 when status_defaults() is executed */ unsigned int exp_table[CLASS_COUNT][2][MAX_LEVEL]; unsigned int max_level[CLASS_COUNT][2]; unsigned int statp[MAX_LEVEL+1]; unsigned int level_penalty[3][RC_MAX][MAX_LEVEL*2+1]; - - unsigned int equip_pos[EQI_MAX]; /* */ struct skill_tree_entry skill_tree[CLASS_COUNT][MAX_SKILL_TREE]; struct fame_list smith_fame_list[MAX_FAME_LIST]; struct fame_list chemist_fame_list[MAX_FAME_LIST]; struct fame_list taekwon_fame_list[MAX_FAME_LIST]; +END_ZEROED_BLOCK; /* End */ + + unsigned int equip_pos[EQI_MAX]; struct sg_data sg_info[MAX_PC_FEELHATE]; /* */ struct eri *sc_display_ers; diff --git a/src/map/skill.c b/src/map/skill.c index 77e9f24aa..70a7a565e 100644 --- a/src/map/skill.c +++ b/src/map/skill.c @@ -18959,16 +18959,7 @@ void skill_readdb(bool minimal) { /* when != it was called during init and this procedure was already performed by skill_defaults() */ if( runflag == MAPSERVER_ST_RUNNING ) { - memset(skill->db,0,sizeof(skill->db) - + sizeof(skill->produce_db) - + sizeof(skill->arrow_db) - + sizeof(skill->abra_db) - + sizeof(skill->magicmushroom_db) - + sizeof(skill->improvise_db) - + sizeof(skill->changematerial_db) - + sizeof(skill->spellbook_db) - + sizeof(skill->reproduce_db) - ); + memset(ZEROED_BLOCK_POS(skill), 0, ZEROED_BLOCK_SIZE(skill)); } // load skill databases @@ -19107,18 +19098,10 @@ void skill_defaults(void) { skill->timer_ers = NULL; skill->cd_ers = NULL; skill->cd_entry_ers = NULL; - /* one huge 0, follows skill.h order */ - memset(skill->db,0,sizeof(skill->db) - + sizeof(skill->produce_db) - + sizeof(skill->arrow_db) - + sizeof(skill->abra_db) - + sizeof(skill->magicmushroom_db) - + sizeof(skill->improvise_db) - + sizeof(skill->changematerial_db) - + sizeof(skill->spellbook_db) - + sizeof(skill->reproduce_db) - + sizeof(skill->unit_layout) - ); + + memset(ZEROED_BLOCK_POS(skill), 0, ZEROED_BLOCK_SIZE(skill)); + memset(skill->unit_layout, 0, sizeof(skill->unit_layout)); + /* */ memcpy(skill->enchant_eff, skill_enchant_eff, sizeof(skill->enchant_eff)); memcpy(skill->deluge_eff, skill_deluge_eff, sizeof(skill->deluge_eff)); diff --git a/src/map/skill.h b/src/map/skill.h index b598b91c8..c5341e9bd 100644 --- a/src/map/skill.h +++ b/src/map/skill.h @@ -1834,6 +1834,7 @@ struct skill_interface { struct eri *cd_ers; // ERS Storage for skill cool down managers [Ind/Hercules] struct eri *cd_entry_ers; // ERS Storage for skill cool down entries [Ind/Hercules] /* */ +BEGIN_ZEROED_BLOCK; // This block will be zeroed in skill_defaults() as well as skill_readdb() struct s_skill_db db[MAX_SKILL_DB]; struct s_skill_produce_db produce_db[MAX_SKILL_PRODUCE_DB]; struct s_skill_arrow_db arrow_db[MAX_SKILL_ARROW_DB]; @@ -1843,6 +1844,7 @@ struct skill_interface { struct s_skill_changematerial_db changematerial_db[MAX_SKILL_PRODUCE_DB]; struct s_skill_spellbook_db spellbook_db[MAX_SKILL_SPELLBOOK_DB]; bool reproduce_db[MAX_SKILL_DB]; +END_ZEROED_BLOCK; struct s_skill_unit_layout unit_layout[MAX_SKILL_UNIT_LAYOUT]; /* */ int enchant_eff[5]; diff --git a/src/map/status.c b/src/map/status.c index b84650b07..c93ea183f 100644 --- a/src/map/status.c +++ b/src/map/status.c @@ -2181,40 +2181,11 @@ int status_calc_pc_(struct map_session_data* sd, enum e_status_calc_opt opt) { sd->def_rate = sd->def2_rate = sd->mdef_rate = sd->mdef2_rate = 100; sd->regen.state.block = 0; - // zeroed arrays, order follows the order in pc.h. - // add new arrays to the end of zeroed area in pc.h (see comments) and size here. [zzo] - memset (sd->param_bonus, 0, sizeof(sd->param_bonus) - + sizeof(sd->param_equip) - + sizeof(sd->subele) - + sizeof(sd->subrace) - + sizeof(sd->subrace2) - + sizeof(sd->subsize) - + sizeof(sd->reseff) - + sizeof(sd->weapon_coma_ele) - + sizeof(sd->weapon_coma_race) - + sizeof(sd->weapon_atk) - + sizeof(sd->weapon_atk_rate) - + sizeof(sd->arrow_addele) - + sizeof(sd->arrow_addrace) - + sizeof(sd->arrow_addsize) - + sizeof(sd->magic_addele) - + sizeof(sd->magic_addrace) - + sizeof(sd->magic_addsize) - + sizeof(sd->magic_atk_ele) - + sizeof(sd->critaddrace) - + sizeof(sd->expaddrace) - + sizeof(sd->ignore_mdef) - + sizeof(sd->ignore_def) - + sizeof(sd->sp_gain_race) - + sizeof(sd->sp_gain_race_attack) - + sizeof(sd->hp_gain_race_attack) -#ifdef RENEWAL - + sizeof(sd->race_tolerance) -#endif - ); + // zeroed arrays + memset(ZEROED_BLOCK_POS(sd), 0, ZEROED_BLOCK_SIZE(sd)); - memset (&sd->right_weapon.overrefine, 0, sizeof(sd->right_weapon) - sizeof(sd->right_weapon.atkmods)); - memset (&sd->left_weapon.overrefine, 0, sizeof(sd->left_weapon) - sizeof(sd->left_weapon.atkmods)); + memset(ZEROED_BLOCK_POS(&(sd->right_weapon)), 0, ZEROED_BLOCK_SIZE(&(sd->right_weapon))); + memset(ZEROED_BLOCK_POS(&(sd->left_weapon)), 0, ZEROED_BLOCK_SIZE(&(sd->left_weapon))); if (sd->special_state.intravision && !sd->sc.data[SC_CLAIRVOYANCE]) //Clear intravision as long as nothing else is using it clif->sc_end(&sd->bl,sd->bl.id,SELF,SI_CLAIRVOYANCE); @@ -2248,40 +2219,6 @@ int status_calc_pc_(struct map_session_data* sd, enum e_status_calc_opt opt) { bstatus->ele_lv = 1; bstatus->race = RC_DEMIHUMAN; - //zero up structures... - memset(&sd->autospell,0,sizeof(sd->autospell) - + sizeof(sd->autospell2) - + sizeof(sd->autospell3) - + sizeof(sd->addeff) - + sizeof(sd->addeff2) - + sizeof(sd->addeff3) - + sizeof(sd->skillatk) - + sizeof(sd->skillusesprate) - + sizeof(sd->skillusesp) - + sizeof(sd->skillheal) - + sizeof(sd->skillheal2) - + sizeof(sd->hp_loss) - + sizeof(sd->sp_loss) - + sizeof(sd->hp_regen) - + sizeof(sd->sp_regen) - + sizeof(sd->skillblown) - + sizeof(sd->skillcast) - + sizeof(sd->add_def) - + sizeof(sd->add_mdef) - + sizeof(sd->add_mdmg) - + sizeof(sd->add_drop) - + sizeof(sd->itemhealrate) - + sizeof(sd->subele2) - + sizeof(sd->skillcooldown) - + sizeof(sd->skillfixcast) - + sizeof(sd->skillvarcast) - + sizeof(sd->skillfixcastrate) - + sizeof(sd->def_set_race) - + sizeof(sd->mdef_set_race) - ); - - memset (&sd->bonus, 0,sizeof(sd->bonus)); - // Autobonus pc->delautobonus(sd,sd->autobonus,ARRAYLENGTH(sd->autobonus),true); pc->delautobonus(sd,sd->autobonus2,ARRAYLENGTH(sd->autobonus2),true); @@ -12427,21 +12364,8 @@ void status_defaults(void) { status->current_equip_item_index = 0; //Contains inventory index of an equipped item. To pass it into the EQUP_SCRIPT [Lupus] status->current_equip_card_id = 0; //To prevent card-stacking (from jA) [Skotlex] - memset(status->max_weight_base,0,sizeof(status->max_weight_base) - + sizeof(status->HP_table) - + sizeof(status->SP_table) - + sizeof(status->aspd_base) - + sizeof(status->Skill2SCTable) - + sizeof(status->IconChangeTable) - + sizeof(status->ChangeFlagTable) - + sizeof(status->SkillChangeTable) - + sizeof(status->RelevantBLTypes) - + sizeof(status->DisplayType) - + sizeof(status->refine_info) - + sizeof(status->atkmods) - + sizeof(status->job_bonus) - + sizeof(status->sc_conf) - ); + // These macros are used instead of a sum of sizeof(), to ensure that padding won't interfere with our size, and code won't rot when adding more fields + memset(ZEROED_BLOCK_POS(status), 0, ZEROED_BLOCK_SIZE(status)); status->data_ers = NULL; memset(&status->dummy, 0, sizeof(status->dummy)); diff --git a/src/map/status.h b/src/map/status.h index 3b85c4014..b370a9c88 100644 --- a/src/map/status.h +++ b/src/map/status.h @@ -1980,7 +1980,8 @@ struct status_interface { /* vars */ int current_equip_item_index; int current_equip_card_id; - /* */ + +BEGIN_ZEROED_BLOCK; /* Everything within this block will be memset to 0 when status_defaults() is executed */ int max_weight_base[CLASS_COUNT]; int HP_table[CLASS_COUNT][MAX_LEVEL + 1]; int SP_table[CLASS_COUNT][MAX_LEVEL + 1]; @@ -1997,6 +1998,8 @@ struct status_interface { int atkmods[3][MAX_WEAPON_TYPE];//ATK weapon modification for size (size_fix.txt) char job_bonus[CLASS_COUNT][MAX_LEVEL]; sc_conf_type sc_conf[SC_MAX]; +END_ZEROED_BLOCK; /* End */ + struct eri *data_ers; //For sc_data entries struct status_data dummy; int64 natural_heal_prev_tick; -- cgit v1.2.3-70-g09d2