Re: [PATCH v2 2/3] drm: Plumb debugfs_init through to panels

From: Andrzej Hajda
Date: Wed Feb 16 2022 - 06:43:04 EST




On 16.02.2022 10:35, Javier Martinez Canillas wrote:
On 2/16/22 10:25, Jani Nikula wrote:

[snip]

I actually wrote said follow-up patches and they were ready to go, but
when I was trying to come up with the right "Fixes" tag I found commit
b792e64021ec ("drm: no need to check return value of debugfs_create
functions"). So what's being requested is nearly the opposite of what
Greg did there.

I thought about perhaps only checking for directories but even that
type of check was removed by Greg's patch. Further checking shows that
start_creating() actually has:

if (IS_ERR(parent))
return parent;


...so I guess that explains why it's fine to skip the check even for parents?

Sure enough I confirmed that if I pass `ERR_PTR(-EINVAL)` as the root
for `panel->funcs->debugfs_init()` that nothing bad seems to happen...
Yeah, the idea is that you don't need to check for debugfs function
return values and you can safely pass error pointers to debugfs
functions. The worst that can happen is you don't get the debugfs, but
hey, it's debugfs so you shouldn't fail anything else because of that
anyway.

Thanks a lot Doug and Jani for the explanations. That makes sense and it
explains why most code I looked was not checking for the return value.

I guess we should write something about this in the debugfs functions
kernel doc so it's mentioned explicitly and people don't have to guess.

Nice, didn't know debugfs started using this pattern. I hope the pattern will be used broader, as it allows to save lot of redundant checks.

Regards
Andrzej



Best regards,