I'm a little confused at the usage of a shared_ptr.. this code works but i'm not sure if a) it's safe or b) it's correct

Decleration:

 std::map<const std::string, std::shared_ptr<ALLEGRO_BITMAP>> BitmapList;
 

Code:

const std::shared_ptr<ALLEGRO_BITMAP> BitmapManager::AddBitmap(const std::string filePath)
{
    std::string fullPath = "Assets/Images/";
    fullPath += filePath.c_str();

    BitmapList[filePath] = std::shared_ptr<ALLEGRO_BITMAP>(al_load_bitmap(fullPath.c_str()));

    if(BitmapList[filePath] != nullptr)
        return BitmapList[filePath];
    else
        return nullptr;
}
 

Usage:

std::shared_ptr<ALLEGRO_BITMAP> b = Bitmap->AddBitmap("test.bmp")
al_draw_bitmap(b.get(), 30, 30, 0)
 

and when compiling in MSVC++ 10 I get these build warnings:

1>c:\program files\microsoft visual studio 10.0\vc\include\memory(1664): warning C4150: deletion of pointer to incomplete type 'ALLEGRO_BITMAP'; no destructor called
1>          c:\allegro\include\allegro5\mouse.h(99) : see declaration of 'ALLEGRO_BITMAP'
1>          c:\program files\microsoft visual studio 10.0\vc\include\memory(1448) : see reference to function template instantiation 'void std::tr1::shared_ptr<_Ty>::_Resetp<_Ty>(_Ux *)' being compiled
1>          with
1>          [
1>              _Ty=ALLEGRO_BITMAP,
1>              _Ux=ALLEGRO_BITMAP
1>          ]
1>          c:\program files\microsoft visual studio 10.0\vc\include\memory(1446) : while compiling class template member function 'std::tr1::shared_ptr<_Ty>::shared_ptr(std::nullptr_t)'
1>          with
1>          [
1>              _Ty=ALLEGRO_BITMAP
1>          ]
1>          \\b-all-data03\data03\2\10544082\documents\visual studio 2010\projects\allegrotest\allegrotest\bitmapmanager.cpp(14) : see reference to class template instantiation 'std::tr1::shared_ptr<_Ty>' being compiled
1>          with
1>          [
1>              _Ty=ALLEGRO_BITMAP
1>          ]
1>c:\program files\microsoft visual studio 10.0\vc\include\memory(1107): warning C4150: deletion of pointer to incomplete type 'ALLEGRO_BITMAP'; no destructor called
1>          c:\allegro\include\allegro5\mouse.h(99) : see declaration of 'ALLEGRO_BITMAP'
1>          c:\program files\microsoft visual studio 10.0\vc\include\memory(1106) : while compiling class template member function 'void std::tr1::_Ref_count<_Ty>::_Destroy(void)'
1>          with
1>          [
1>              _Ty=ALLEGRO_BITMAP
1>          ]
1>          c:\program files\microsoft visual studio 10.0\vc\include\memory(1662) : see reference to class template instantiation 'std::tr1::_Ref_count<_Ty>' being compiled
1>          with
1>          [
1>              _Ty=ALLEGRO_BITMAP
1>          ]
1>          c:\program files\microsoft visual studio 10.0\vc\include\memory(1448) : see reference to function template instantiation 'void std::tr1::shared_ptr<_Ty>::_Resetp<_Ty>(_Ux *)' being compiled
1>          with
1>          [
1>              _Ty=ALLEGRO_BITMAP,
1>              _Ux=ALLEGRO_BITMAP
1>          ]

BitmapList[filePath] = std::shared_ptr<ALLEGRO_BITMAP>(al_load_bitmap(fullPath.c_str()), [] (ALLEGRO_BITMAP* bm) { al_delete_bitmap(bm); });

Ah that works, thanks BitMaster and thanks for the comments Bregma :)

(1) Always use std::make_shared where you can, instead of the new operator.
BitmapList[filePath] = std::make_shared<ALLEGRO_BITMAP>(al_load_bitmap(fullPath.c_str()));

(2) Do not return nullptr when you mean to return std::shared_ptr() -- they're not al all the same thing. That's probably the source of your trouble.

(1) Always use std::make_shared where you can, instead of the new operator.
BitmapList[filePath] = std::make_shared<ALLEGRO_BITMAP>(al_load_bitmap(fullPath.c_str()));

I don't see the point of this. The idea behind std::make_shared is to avoid two allocations for new (one for the shared_ptr control structure, one for the actual object), but in the case of al_load_bitmap you are completely unable to combine allocations. Looking at the warnings the OP gets, the ALLEGRO_BITMAP type is completely opaque. Setting aside it needs to be allocated in a different runtime (whichever runtime the Allegro library uses), you don't know anything about it, not even the size.

std::make_shared is also for exception safety, though it's not needed here. But I'd say using make_* is a good habit to have.

(2) Do not return nullptr when you mean to return std::shared_ptr() -- they're not al all the same thing. That's probably the source of your trouble.

I don't agree with this. The code does the same thing either way. The source of the trouble is that ALLEGRO_BITMAP is an incomplete type.

(2) Do not return nullptr when you mean to return std::shared_ptr() -- they're not al all the same thing. That's probably the source of your trouble.

I don't agree with this. The code does the same thing either way. The source of the trouble is that ALLEGRO_BITMAP is an incomplete type.

Actually, while the incomplete type is indeed the root cause of the problem here (and also the reason std::make_shared is a no-go), it's often a bad idea to construct any shared_ptr using a null_ptr (or 0 for boost::shared_ptr in pre-C++11). When you construct a shared_ptr with a nullptr instead of default constructing it, the deleter will be called when the object is freed. That's not an issue with the default deleter because delete null_ptr is legal, other deleters might not have this property (something I learned the hard way with GDAL objects).