Re: [PATCH 2/2] scsi: scsi_debug: delete some bogus error checking

[Wenchao Hao <haowenchao22@gmail.com>]

On 2023/10/20 22:15, Dan Carpenter wrote:
> Smatch complains that "dentry" is never initialized.  These days everyone
> initializes all their stack variables to zero so this means that it will
> trigger a warning every time this function is run.
> 
> Really debugfs functions are not supposed to be checked for errors so
> this checking can just be deleted.
> 
> Fixes: f084fe52c640 ("scsi: scsi_debug: Add debugfs interface to fail target reset")
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
> See my blog for more information on the history of debugfs error
> checking:
> 
> https://staticthinking.wordpress.com/2023/07/24/debugfs-functions-are-not-supposed-to-be-checked/
> ---
>  drivers/scsi/scsi_debug.c | 7 -------
>  1 file changed, 7 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index 0a4e41d84df8..c0be9a53ac79 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -1127,7 +1127,6 @@ static const struct file_operations sdebug_target_reset_fail_fops = {
>  static int sdebug_target_alloc(struct scsi_target *starget)
>  {
>  	struct sdebug_target_info *targetip;
> -	struct dentry *dentry;
>  
>  	targetip = kzalloc(sizeof(struct sdebug_target_info), GFP_KERNEL);
>  	if (!targetip)
> @@ -1135,15 +1134,9 @@ static int sdebug_target_alloc(struct scsi_target *starget)
>  
>  	targetip->debugfs_entry = debugfs_create_dir(dev_name(&starget->dev),
>  				sdebug_debugfs_root);
> -	if (IS_ERR_OR_NULL(targetip->debugfs_entry))
> -		pr_info("%s: failed to create debugfs directory for target %s\n",
> -			__func__, dev_name(&starget->dev));
>  
>  	debugfs_create_file("fail_reset", 0600, targetip->debugfs_entry, starget,
>  				&sdebug_target_reset_fail_fops);
> -	if (IS_ERR_OR_NULL(dentry))
> -		pr_info("%s: failed to create fail_reset file for target %s\n",
> -			__func__, dev_name(&starget->dev));
>  
>  	starget->hostdata = targetip;
>  


Thank you for the fix, the check for debugfs_create_file() is added because 
scsi_debug driver is often used to test abnormal situations, here just check
and prompt a log, so maybe you should not remove it and fix the issue
following changes:

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 67922e2c4c19..d37437fa9eba 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -1144,8 +1144,8 @@ static int sdebug_target_alloc(struct scsi_target *starget)
                pr_info("%s: failed to create debugfs directory for target %s\n",
                        __func__, dev_name(&starget->dev));
 
-       debugfs_create_file("fail_reset", 0600, targetip->debugfs_entry, starget,
-                               &sdebug_target_reset_fail_fops);
+       dentry = debugfs_create_file("fail_reset", 0600, targetip->debugfs_entry,
+                                    starget, &sdebug_target_reset_fail_fops);
        if (IS_ERR_OR_NULL(dentry))
                pr_info("%s: failed to create fail_reset file for target %s\n",
                        __func__, dev_name(&starget->dev));