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

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

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@ooseel.net>
> ---
>  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?

thanks,

greg k-h