On 09/11/2018 12:15 PM, Dan Carpenter wrote:
> On Tue, Sep 11, 2018 at 12:11:23PM +0300, Alexey Skidanov wrote:
>>
>>
>> On 09/11/2018 11:59 AM, Greg KH wrote:
>>> On Tue, Sep 11, 2018 at 11:50:19AM +0300, Dan Carpenter wrote:
>>>> On Tue, Sep 11, 2018 at 11:17:10AM +0300, Alexey Skidanov wrote:
>>>>> @@ -546,6 +556,38 @@ void ion_device_add_heap(struct ion_heap *heap)
>>>>> }
>>>>>
>>>>> heap->dev = dev;
>>>>> + heap->num_of_buffers = 0;
>>>>> + heap->num_of_alloc_bytes = 0;
>>>>> + heap->alloc_bytes_wm = 0;
>>>>> +
>>>>> + /* Create heap root directory. If creation is failed, we may continue */
>>>>> + heap_root = debugfs_create_dir(heap->name, dev->debug_root);
>>>>> + if (!IS_ERR_OR_NULL(heap_root)) {
>>>>
>>>> Just remove this check. If debugfs_create_dir() fails, it doesn't
>>>> cause any problems here.
>> If debugfs_create_dir fails, the heap_root will be either NULL (and thus
>> the underlying files will be created under /sys/kernel/debug) or -EXXX
>> (and thus we probably crash the Kernel). Am I wrong?
>
> Yes. You're wrong. It's fine.
>
> It's designed to normally work without error handling. Some drivers
> dereference "heap_root" so they would need to check, but if you're not
> dereferencing it yourself, then debugfs checks for NULL. (It doesn't
> check for error pointer because in that situation debugfs functions
> are all no-ops).
>
> regards,
> dan carpenter
>
Agree with you regarding the error case. But in case of NULL - the
underlying files will be created in the wrong path. So probably it makes
sense to check it for NULL only?
Thanks,
Alexey
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
