Re: Cppcheck: throwing exception in destructor in SyncDbusSessionHelper.cxx (shell module)

On 05/12/2013 04:19 PM, julien2412 wrote:
> Cppcheck reported exception thrown in destructor in
> shell/source/sessioninstall/SyncDbusSessionHelper.cxx, line 39
>       28     class GErrorWrapper
>       29     {
>       30         GError* m_pError;
>       31         public:
>       32             GErrorWrapper(GError* pError) : m_pError(pError) {}
>       33             ~GErrorWrapper()
>       34             {
>       35                 if(!m_pError)
>       36                     return;
>       37                 OUString sMsg =
> OUString::createFromAscii(m_pError->message);
>       38                 g_error_free(m_pError);
>       39                 throw RuntimeException(sMsg, NULL);
>       40             }
>       41             GError** getRef() { return &m_pError; }
>       42     };
>
> Should it be changed? If yes, how? (I suppose we shouldn't just remove it)

So this uses the RAII idiom to ensure one does not forget to call 
g_error_free, and consequently mis-uses the idiom to also throw from a 
dtor a RuntimeException synthesized from the GError.

In principle, this could be safe, if it can be proven that no stack 
unwinding out of the scope of a GErorrWrapper instance can happen. 
However, already the first use in lcl_GetPackageKitProxy shows that this 
is too error prone---the "throw RuntimeException if !proxy" is covered 
by the GErrorWrapper instance's scope, so this will likely lead to abort.

So the best is probably to replace RAII GErrorWrapper with something 
"less clever" that doesn't require throwing from a dtor.

Stephan