From 37767f6505db9e7a4039621cfd77623b06d92606 Mon Sep 17 00:00:00 2001 From: Ibrahim Zidan Date: Mon, 8 Apr 2019 04:23:08 +0200 Subject: Rewrite clif_storageItems - The maximum packetsize is now decided during compile time depending on client version which fixes an issue started with clients supporting int32 as itemid where packet size would underflow - The function now have a single loop that is easier to read and understand Signed-off-by: Ibrahim Zidan --- src/map/clif.c | 42 ++++++++++++++++++------------------------ src/map/clif.h | 5 +++++ 2 files changed, 23 insertions(+), 24 deletions(-) (limited to 'src') diff --git a/src/map/clif.c b/src/map/clif.c index cd3131181..0f8469e64 100644 --- a/src/map/clif.c +++ b/src/map/clif.c @@ -2977,28 +2977,22 @@ static void clif_storageItems(struct map_session_data *sd, enum inventory_type t nullpo_retv(sd); nullpo_retv(items); - int i = 0; - struct item_data *id; - - do { - int normal = 0, equip = 0, k = 0; - - for( ; i < items_length && k < 500; i++, k++ ) { + for (int i = 0, normal_count = 0, equip_count = 0; i < items_length; ++i) { + struct item_data *itd; - if( items[i].nameid <= 0 ) - continue; + if (items[i].nameid == 0) + continue; - id = itemdb->search(items[i].nameid); + itd = itemdb->search(items[i].nameid); - if( !itemdb->isstackable2(id) ) //Non-stackable (Equippable) - clif->item_equip(i+1,&storelist_equip.list[equip++],&items[i],id,id->equip); - else //Stackable (Normal) - clif->item_normal(i+1,&storelist_normal.list[normal++],&items[i],id); - } + if (!itemdb->isstackable2(itd)) + clif->item_equip(i + 1, &storelist_equip.list[equip_count++], &items[i], itd, itd->equip); + else + clif->item_normal(i + 1, &storelist_normal.list[normal_count++], &items[i], itd); - if( normal ) { - storelist_normal.PacketType = storageListNormalType; - storelist_normal.PacketLength = ( sizeof( storelist_normal ) - sizeof( storelist_normal.list ) ) + (sizeof(struct NORMALITEM_INFO) * normal); + if (normal_count > 0 && (normal_count == MAX_STORAGE_ITEM_PACKET_NORMAL || i + 1 == items_length)) { + storelist_normal.PacketType = storageListNormalType; + storelist_normal.PacketLength = (sizeof(storelist_normal) - sizeof(storelist_normal.list)) + (sizeof(struct NORMALITEM_INFO) * normal_count); #if PACKETVER_RE_NUM >= 20180912 || PACKETVER_ZERO_NUM >= 20180919 || PACKETVER_MAIN_NUM >= 20181002 storelist_normal.invType = type; @@ -3008,11 +3002,12 @@ static void clif_storageItems(struct map_session_data *sd, enum inventory_type t #endif clif->send(&storelist_normal, storelist_normal.PacketLength, &sd->bl, SELF); + normal_count = 0; } - if( equip ) { - storelist_equip.PacketType = storageListEquipType; - storelist_equip.PacketLength = ( sizeof( storelist_equip ) - sizeof( storelist_equip.list ) ) + (sizeof(struct EQUIPITEM_INFO) * equip); + if (equip_count > 0 && (equip_count == MAX_STORAGE_ITEM_PACKET_EQUIP || i + 1 == items_length)) { + storelist_equip.PacketType = storageListEquipType; + storelist_equip.PacketLength = (sizeof(storelist_equip) - sizeof(storelist_equip.list)) + (sizeof(struct EQUIPITEM_INFO) * equip_count); #if PACKETVER_RE_NUM >= 20180912 || PACKETVER_ZERO_NUM >= 20180919 || PACKETVER_MAIN_NUM >= 20181002 storelist_equip.invType = type; @@ -3022,10 +3017,9 @@ static void clif_storageItems(struct map_session_data *sd, enum inventory_type t #endif clif->send(&storelist_equip, storelist_equip.PacketLength, &sd->bl, SELF); + equip_count = 0; } - - } while ( i < items_length ); - + } } static void clif_cartList(struct map_session_data *sd) diff --git a/src/map/clif.h b/src/map/clif.h index 6c9058cba..b3441f908 100644 --- a/src/map/clif.h +++ b/src/map/clif.h @@ -78,6 +78,11 @@ enum rodex_get_items; #define COLOR_YELLOW 0xffff00U #define COLOR_DEFAULT COLOR_GREEN +#define MAX_STORAGE_ITEM_PACKET_NORMAL ((INT16_MAX - (sizeof(struct ZC_STORE_ITEMLIST_NORMAL) - (sizeof(struct NORMALITEM_INFO) * MAX_ITEMLIST))) / sizeof(struct NORMALITEM_INFO)) +#define MAX_STORAGE_ITEM_PACKET_EQUIP ((INT16_MAX - (sizeof(struct ZC_STORE_ITEMLIST_EQUIP) - (sizeof(struct EQUIPITEM_INFO) * MAX_ITEMLIST))) / sizeof(struct EQUIPITEM_INFO)) +STATIC_ASSERT(MAX_STORAGE_ITEM_PACKET_NORMAL > 0, "Max items per storage item packet for normal items is less than 1, it's most likely to be a bug and shall not be ignored."); +STATIC_ASSERT(MAX_STORAGE_ITEM_PACKET_EQUIP > 0, "Max items per storage item packet for equip items is less than 1, it's most likely to be a bug and shall not be ignored."); + /** * Enumerations **/ -- cgit v1.2.3-70-g09d2 From 5410de9d04124ce34f58173d616c21088a1482ee Mon Sep 17 00:00:00 2001 From: Ibrahim Zidan Date: Mon, 8 Apr 2019 04:37:35 +0200 Subject: Addition of missing nullpo check in clif_send and an assertion check for zero/negatvie length values Signed-off-by: Ibrahim Zidan --- src/map/clif.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'src') diff --git a/src/map/clif.c b/src/map/clif.c index 0f8469e64..53c2c6b23 100644 --- a/src/map/clif.c +++ b/src/map/clif.c @@ -431,8 +431,13 @@ static int clif_send_actual(int fd, void *buf, int len) *------------------------------------------*/ static bool clif_send(const void *buf, int len, struct block_list *bl, enum send_target type) { + if (type != ALL_CLIENT) + nullpo_retr(false, bl); + nullpo_retr(false, buf); + Assert_retr(false, len > 0); + int i; - struct map_session_data *sd, *tsd; + struct map_session_data *sd = BL_CAST(BL_PC, bl), *tsd; struct party_data *p = NULL; struct guild *g = NULL; struct battleground_data *bgd = NULL; @@ -440,11 +445,6 @@ static bool clif_send(const void *buf, int len, struct block_list *bl, enum send struct s_mapiterator* iter; int area_size; - if( type != ALL_CLIENT ) - nullpo_ret(bl); - - sd = BL_CAST(BL_PC, bl); - if (sd != NULL && pc_isinvisible(sd)) { if (type == AREA || type == BG || type == BG_AREA) type = SELF; -- cgit v1.2.3-70-g09d2 From 112b19df8acb95e5599927fc4b505d4ca2e610a4 Mon Sep 17 00:00:00 2001 From: Ibrahim Zidan Date: Mon, 8 Apr 2019 04:42:12 +0200 Subject: Fix direct calls to clif_send in clif.c Signed-off-by: Ibrahim Zidan --- src/map/clif.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) (limited to 'src') diff --git a/src/map/clif.c b/src/map/clif.c index 53c2c6b23..38d0215d3 100644 --- a/src/map/clif.c +++ b/src/map/clif.c @@ -2978,12 +2978,10 @@ static void clif_storageItems(struct map_session_data *sd, enum inventory_type t nullpo_retv(items); for (int i = 0, normal_count = 0, equip_count = 0; i < items_length; ++i) { - struct item_data *itd; - if (items[i].nameid == 0) continue; - itd = itemdb->search(items[i].nameid); + struct item_data *itd = itemdb->search(items[i].nameid); if (!itemdb->isstackable2(itd)) clif->item_equip(i + 1, &storelist_equip.list[equip_count++], &items[i], itd, itd->equip); @@ -7125,7 +7123,7 @@ static void clif_party_job_and_level(struct map_session_data *sd) WBUFW(buf, 6) = sd->status.class; WBUFW(buf, 8) = sd->status.base_level; - clif_send(buf, packet_len(0xabd), &sd->bl, PARTY); + clif->send(buf, packet_len(0xabd), &sd->bl, PARTY); #endif } @@ -21630,7 +21628,7 @@ static void clif_hat_effect_single(struct block_list *bl, uint16 effectId, bool WBUFB(buf,8) = enable; WBUFL(buf,9) = effectId; - clif_send(buf, 13, bl, AREA); + clif->send(buf, 13, bl, AREA); #endif } -- cgit v1.2.3-70-g09d2