CreateItem register işleminden sonra SetCount kontrolü

[Larry Watterson Telefon numaranız başarıyla doğrulandı.]

Olay şu;

item->SetVID(++m_dwVIDCount);

if (bSkipSave == false)
    m_VIDMap.emplace(item->GetVID(), item);

if (item->GetID() != 0 && bSkipSave == false)
    m_map_pkItemByID.emplace(item->GetID(), item);

if (!item->SetCount(count)) // LOL
    return NULL;

item "SetCOunt" member funcın dönüş değeri kontrol edilmeden VID değeri alıyor ve 2 adet map'e emplace ediliyor. "SetCount" başarısız olursa fonksiyon NULL return ediyor ama item halen iki haritada bulunuyor, sonrasında herhangi bir yerde silinmiyor(DestroyItem çağrılması gerekiyor).

Şu anlık bir sorun teşkil etmiyor çünkü "SetCount" funcının başarısızlık durumu countun 0 ve owner'ın nullptr olmasına bağlı yani kısaca cold path. Mantıksal akış ve ileriye dönük amaçlı düzeltebilirsiiz, buna ek olarak "SetCount" implementasyonunda düzenleme yaptıysanız dikkatli olmanız gerekebilir.

Şu şekilde düzeltebilirsiniz;

if (!item->SetCount(count)) [[unlikely]]
    return nullptr;

item->SetVID(++m_dwVIDCount);

if (bSkipSave == false)
    m_VIDMap.emplace(item->GetVID(), item);

if (item->GetID() != 0 && bSkipSave == false)
    m_map_pkItemByID.emplace(item->GetID(), item);

 

[Silverhand]

Ellerine sağlık @Larry Watterson

[[unlikely]] C++20 attribute’ü. Konudaki düzeltmeyi uygulamayacak arkadaşlar Source’u C++11/14 ile derliyorsanız compiler desteğine göre sorun çıkarabilir.

Aktif olarak tetiklenen bir bug değil. Daha çok mantıksal akışın düzeltilmesi ve ileride SetCount() implementasyonu değiştirilirse oluşabilecek problemlerin önlenmesi açısından yapılabilecek bir düzenleme gibi duruyor.

if (!item->SetCount(count))
    return nullptr;

item->SetVID(++m_dwVIDCount);

if (!bSkipSave)
{
    m_VIDMap.insert(ITEM_VID_MAP::value_type(item->GetVID(), item));

    if (item->GetID() != 0)
        m_map_pkItemByID.insert(
            std::map<DWORD, LPITEM>::value_type(item->GetID(), item));
}

 

[mgurbet1998]

konuya ve yoruma yanıt vericektim ama tokenlerim bitmiş dolsun gelcem.

 

[Larry Watterson Telefon numaranız başarıyla doğrulandı.]

Ellerine sağlık @Larry Watterson

[[unlikely]] C++20 attribute’ü. Konudaki düzeltmeyi uygulamayacak arkadaşlar Source’u C++11/14 ile derliyorsanız compiler desteğine göre sorun çıkarabilir.

Aktif olarak tetiklenen bir bug değil. Daha çok mantıksal akışın düzeltilmesi ve ileride SetCount() implementasyonu değiştirilirse oluşabilecek problemlerin önlenmesi açısından yapılabilecek bir düzenleme gibi duruyor.

if (!item->SetCount(count))
    return nullptr;

item->SetVID(++m_dwVIDCount);

if (!bSkipSave)
{
    m_VIDMap.insert(ITEM_VID_MAP::value_type(item->GetVID(), item));

    if (item->GetID() != 0)
        m_map_pkItemByID.insert(
            std::map<DWORD, LPITEM>::value_type(item->GetID(), item));
}

ya burada zaten red-black tree DS kullanmak bile yanlış bir tercih, implementasyon, tercihler, API, kullanım senaryoları, koşullar vs. üzerine konuşulabilecek onlarca şey var ama bunlar konunun kapsamı dışında olan şeyler, hiçbir şeyi düşünmeden evet, yazdığınız gibi kullanılabilir

 

[Okan Çakın]

M2_DELETE(item); neden yok merak ettim. Orjinal altyapıda kendiliğinden smart pointere mi geçmiş yoksa cinler mi geçirmiş.