Re: [PATCH] android: binder: print error message on failure of creating proc file

[Leesoo Ahn]

2024년 7월 12일 (금) 오후 2:40, Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>님이 작성:
>
> On Fri, Jul 12, 2024 at 12:21:40PM +0900, Leesoo Ahn wrote:
> > It better prints out an error message to give more information if
> > calling debugfs_create_file() is failure and the return value has an
> > error code.
> >
> > Signed-off-by: Leesoo Ahn <lsahn@xxxxxxxxxx>
> > ---
> >  drivers/android/binder.c | 12 +++++++++++-
> >  1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> > index b21a7b246a0d..eb0fd1443d69 100644
> > --- a/drivers/android/binder.c
> > +++ b/drivers/android/binder.c
> > @@ -5673,6 +5673,7 @@ static int binder_open(struct inode *nodp, struct file *filp)
> >
> >       if (binder_debugfs_dir_entry_proc && !existing_pid) {
> >               char strbuf[11];
> > +             struct dentry *debugfs_entry;
> >
> >               snprintf(strbuf, sizeof(strbuf), "%u", proc->pid);
> >               /*
> > @@ -5681,10 +5682,19 @@ static int binder_open(struct inode *nodp, struct file *filp)
> >                * The printing code will anyway print all contexts for a given
> >                * PID so this is not a problem.
> >                */
> > -             proc->debugfs_entry = debugfs_create_file(strbuf, 0444,
> > +             debugfs_entry = debugfs_create_file(strbuf, 0444,
> >                       binder_debugfs_dir_entry_proc,
> >                       (void *)(unsigned long)proc->pid,
> >                       &proc_fops);
> > +             if (!IS_ERR(debugfs_entry)) {
> > +                     proc->debugfs_entry = debugfs_entry;
> > +             } else {
> > +                     int error;
> > +
> > +                     error = PTR_ERR(debugfs_entry);
> > +                     pr_warn("Unable to create file %s in debugfs (error %d)\n",
> > +                             strbuf, error);
>
> Even if we wanted to warn about this (hint, you don't, see previous
> response), this way to check is incorrect and will fail if debugfs is
> not enabled, which you don't want to have happen.
>
> So I'm guessing you did not test this with that config option disabled?

Oh, I haven't thought about this and just figured out it would work weird.
Thank you for mentioning it.

>
> thanks,
>
> greg k-h