> I notice that your editor is using tab stops.  Can you configure it to use
> spaces instead of tabs?  See the diff for
> plugins/unityshell/src/DeviceLauncherSection.cpp to see what I see.

Of course.
 
> If you are going to the trouble of adding an anonymous namespace, then you
> should replace the #define:
> 
> plugins/unityshell/src/DeviceLauncherIcon.cpp
> 
> namespace {
> const char* DEFAULT_ICON = "drive-removable-media";
> }

Of course.
 
>   DeviceLauncherIcon::DeviceLauncherIcon(Launcher *launcher, GVolume *volume)
> should be:
>   DeviceLauncherIcon::DeviceLauncherIcon(Launcher* launcher, GVolume* volume)
> 
> Keep the pointer with the type not the variable name.

I don't like it, but it will be done ;)
 
> > void DeviceLauncherIcon::UpdateDeviceIcon()
> 
> The extra scoping you have in this method don't really help the readability,
> nor are they required for early destruction of the objects.  Can you remove
> them please?
> 

Sure. They are not my method.

> There is no point at all in adding the "inline" keyword to a function in the
> source file.  Inlining only works if the compiler knows at the time the other
> source file is being compiled (at least I'm fairly sure about that).
>

Ops...

> > std::list<DbusmenuMenuitem *> DeviceLauncherIcon::GetMenus()
> 
> I'm not a fan of returning containers from methods.  Normally there is a
> better way to structure the program.  You don't need to do anything about this
> at this stage, just somethingn to consider for the future.
> 
> Although I just noticed that the list is containing things that you explicitly
> create. This means that every time you call this method you need to remember
> to explicitly free all of the objects.  Hmm... I also see that you didn't
> create this method.  I'll chase this up elsewhere.
> 

I can do nothing here. This is the quicklist manager if i am not wrong. Let me know
what can i do here.

> > void DeviceLauncherIcon::ShowMount(GMount *mount)
> 
> Please keep the pointer with the type.
> 
> > g_mount_unmount_with_operation(mount.RawPtr(),
> 
> This should work without the .RawPtr() call.  The object implements an
> operator OBJECT* method, and since the g_mount_unmount_with_operation expects
> a GMount, the method is used.
>

I use this because sometimes i got warnings.

> Same for g_drive_stop.
> Also try for the g_signal_connect.  It should work.  Let me know if it
> doesn't.
> 

I will try...

> std::map<GVolume *, DeviceLauncherIcon *> is screaming for a typedef.  The
> whole using a map based on pointers makes me queezy.  I don't feel comfortable
> that the contained values are destroyed consistently.  The are newed when
> added to the map, but removal just calls erase.  I don't see a corresponding
> delete.  Instead of storing raw pointers, could we key it off a consistent
> name perhaps?  And store smart pointers?

Instead of <GVolume*> we could use a uuid but according to me is too slow... For the key
(GVolume *) we don't need a g_object_unref(...).
For what regard the value (DeviceLauncherIcon *) we cannot use a smart pointer (i used a boost one) because when we OnVolumeRemoved is called, we call "self->map_[volume]->OnRemoved();" that call IconLauncher::Remove() that emit a signal that is handled by LancherController.cpp or LauncherModels.cpp (I don't remember! :) ) that unref the icon... If we use a smartpointer we "unref" it twice... Correct me if I'm wrong.