thx T



On 29/03/10 18:48, Ted Gould wrote:
> Review: Approve
>    review approve
>
> On Mon, 2010-03-29 at 17:28 +0000, Conor Curran wrote:
>    
>> Conor Curran has proposed merging lp:~cjcurran/indicator-sound/crash_sink_removal into lp:indicator-sound.
>>
>> Requested reviews:
>>    Indicator Applet Developers (indicator-applet-developers)
>> Related bugs:
>>    #522428 indicator-sound-service crashed with SIGSEGV in pa_pdispatch_run()
>>    https://bugs.launchpad.net/bugs/522428
>>
>>
>> Tidies up the whole area of hotswapping audio devices. Before I was leaving old invalid sink data in the sink_hash - now it is removed and destroyed.
>> also now the update_sink_info callback doesn't assume the info pointer is valid. (Bug #522428).
>>
>> differences between files attachment (review-diff.txt)
>> === modified file 'src/pulse-manager.c'
>> --- src/pulse-manager.c	2010-03-25 10:35:30 +0000
>> +++ src/pulse-manager.c	2010-03-29 17:28:23 +0000
>> @@ -67,7 +67,6 @@
>>
>>       // Establish event callback registration
>>   	pa_context_set_state_callback(pulse_context, context_state_callback, NULL);
>> -    // Broadcast init state (assume we have a device - if not the signals will handle it)
>>       dbus_menu_manager_update_pa_state(FALSE, FALSE, FALSE, 0);
>>   	pa_context_connect(pulse_context, NULL, PA_CONTEXT_NOFAIL, NULL);
>>   }
>> @@ -359,9 +358,14 @@
>>           g_warning("Sink INPUT info callback failure");
>>           return;
>>       }
>> -
>> +    gint position = -1;
>>       GList *keys = g_hash_table_get_keys(sink_hash);
>> -    gint position =  g_list_index(keys, GINT_TO_POINTER(info->index));
>> +
>> +    if(info == NULL)
>> +        return;
>> +
>> +    position =  g_list_index(keys, GINT_TO_POINTER(info->index));
>> +
>>       if(position>= 0) // =>  index is within the keys of the hash.
>>       {
>>           sink_info *s = g_hash_table_lookup(sink_hash, GINT_TO_POINTER(info->index));
>> @@ -459,11 +463,14 @@
>>               {
>>                   if(index == DEFAULT_SINK_INDEX){
>>                       g_debug("PA_SUBSCRIPTION_EVENT_SINK REMOVAL event triggered - default sink has been removed !! \n updating UI to reflect the change");
>> -                    sound_service_dbus_update_sink_availability(dbus_service, FALSE);
>> +                    gboolean availability = determine_sink_availability();
>> +                    sound_service_dbus_update_sink_availability(dbus_service, availability);
>>                   }
>>                   else{
>>                       g_debug("PA_SUBSCRIPTION_EVENT_SINK REMOVAL - some device other than the default - no panic");
>>                   }
>> +                g_debug("removing sink of index %i from our sink hash - keep the cache tidy !", index);
>> +                g_hash_table_remove(sink_hash, GINT_TO_POINTER(index));
>>               }
>>               else
>>               {
>> @@ -474,7 +481,7 @@
>>   			g_debug("PA_SUBSCRIPTION_EVENT_SINK_INPUT event triggered!!");
>>               if ((t&  PA_SUBSCRIPTION_EVENT_TYPE_MASK) == PA_SUBSCRIPTION_EVENT_REMOVE)
>>               {
>> -                //handle the remove event - not relevant for current design
>> +                //handle the sink input remove event - not relevant for current design
>>               }
>>               else
>>               {			
>>
>>      
>
>    


-- 
Conor Curran
Desktop Architect Sound Engineer
Desktop Experience team
Canonical Ltd
Email: <email address hidden>
web: http://www.canonical.com