Re: [PATCH] ath10k: Fix un-initialized debug objects.

[Ben Greear <greearb@candelatech.com>]

On 10/28/2013 03:10 AM, Kalle Valo wrote:
> Ben Greear <greearb@candelatech.com> writes:
> 
>> On 10/25/2013 08:58 PM, Kalle Valo wrote:
>>> Ben Greear <greearb@candelatech.com> writes:
>>>
>>>> On 10/25/2013 12:49 AM, Kalle Valo wrote:
>>>>> greearb@candelatech.com writes:
>>>>
>>>>>>    	INIT_WORK(&ar->restart_work, ath10k_core_restart);
>>>>>>
>>>>>> +	ath10k_debug_init(ar);
>>>>>
>>>>> For symmetry would it make more sense to move ath10k_debug_destroy() to
>>>>> ath10k_core_unregister()? That way we could avoid adding a new function.
>>>>>
>>>>
>>>> In my opinion, you should initialize such things as early as possible
>>>> so you don't have to worry so much about the various error cases leaving
>>>> things un-initialized.  I believe my patch accomplished that.
>>>
>>> Well, I again like symmetry and simplicity.
>>>
>>>> What new function are you planning to avoid?
>>>
>>> I would prefer not to create ath10k_debug_init().
>>>
>>> What I was trying to suggest is this, I think it should fix the bug you
>>> are seeing:
>>
>> I don't think so.  The problem is that the init logic can fail before
>> the debug logic is ever initialized, so any cleanup that attempts to
>> access the debug logic can blow up.
> 
> Please look again. My idea was to fix the logic so that
> ath10k_debug_destroy() is called only if ath10k_debug_create() succeeds.

Ok, I think your patch will work.  I'll back mine out when you push
yours and will let you know if I find any more problems in this
area.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k