On 9/21/2016 6:15 PM, Vladyslav Buslov wrote:
>> On 9/20/2016 7:36 PM, Stephen Hemminger wrote:
>>> On Tue, 20 Sep 2016 21:16:37 +0300
>>> Vladyslav Buslov <vladyslav.buslov at harmonicinc.com> wrote:
>>>
>>>> @@ -123,6 +125,9 @@ static int __net_init kni_init_net(struct net *net)
>>>> /* Clear the bit of device in use */
>>>> clear_bit(KNI_DEV_IN_USE_BIT_NUM, &knet->device_in_use);
>>>>
>>>> + mutex_init(&knet->kni_kthread_lock);
>>>> + knet->kni_kthread = NULL;
>>>> +
>>>
>>> Why not just use kzalloc() here? You would still need to init the
>>> mutex etc, but it would be safer.
>>>
>>
>> Hi Vladyslav,
>>
>> This is good suggestion, if you send a new version for this update, please
>> keep my Ack.
>>
>> Thanks,
>> ferruh
>
> Hi Ferruh, Stephen,
>
> Could you please elaborate on using kzalloc for this code.
> Currently kni_thread_lock is value member of kni_net structure and never
> explicitly allocated or deallocated.
> Kni_kthread is pointer member of kni_net and is implicitly created and
> destroyed by kthread_run, kthread_stop functions.
> Which one of those do you suggest to allocate with kzalloc() and how would it
> improve safety?
>
Currently:
kni_init_net() {
knet = kmalloc(..);
..
mutex_init(..);
knet->kni_thread = NULL;
}
If you allocate knet via kzalloc(), no need to assign NULL to
kni_thread. Also this is safer because any uninitialized knet field will
be zero instead of random value.
This is what I understood at least J
Thanks,
ferruh
