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

_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to