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

[Kalle Valo <kvalo@qca.qualcomm.com>]

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.

> All my patch does is ensure the debug related members are always initialized,
> and so can be cleaned up later without problem.

It adds an unnecessary function which is not needed AFAICS.

-- 
Kalle Valo

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