| Message ID | c602c9ad-5e35-4e18-a47f-87ed956a9ec2@moroto.mountain (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v2,1/2] scsi: scsi_debug: scsi: scsi_debug: fix some bugs in sdebug_error_write() | expand |
On 2023/11/6 22:05, 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 in > normal code. For example, if we updated this code to check the correct > variable then it would print a warning if CONFIG_DEBUGFS was disabled. > We don't want that. Just delete the check. > > Fixes: f084fe52c640 ("scsi: scsi_debug: Add debugfs interface to fail target reset") > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> > --- > v2: Add some more text to the commit message about CONFIG_DEBUGFS > > 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 0dd21598f7b6..6d8218a44122 100644 > --- a/drivers/scsi/scsi_debug.c > +++ b/drivers/scsi/scsi_debug.c > @@ -1132,7 +1132,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) > @@ -1140,15 +1139,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; > Looks good, thanks for the fix. Reviewed-by: Wenchao Hao <haowenchao2@huawei.com>
diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c index 0dd21598f7b6..6d8218a44122 100644 --- a/drivers/scsi/scsi_debug.c +++ b/drivers/scsi/scsi_debug.c @@ -1132,7 +1132,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) @@ -1140,15 +1139,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;
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 in normal code. For example, if we updated this code to check the correct variable then it would print a warning if CONFIG_DEBUGFS was disabled. We don't want that. Just delete the check. Fixes: f084fe52c640 ("scsi: scsi_debug: Add debugfs interface to fail target reset") Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> --- v2: Add some more text to the commit message about CONFIG_DEBUGFS drivers/scsi/scsi_debug.c | 7 ------- 1 file changed, 7 deletions(-)