| Message ID | fa2b2c9c72335ab4c3d5e6a33415e7f020b1d51b.1620243401.git.christophe.jaillet@wanadoo.fr (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | cxl: Fix an error message | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (7619d98e5041d5c25aba5428704dba6121237a9a) |
| snowpatch_ozlabs/build-ppc64le | success | Build succeeded |
| snowpatch_ozlabs/build-ppc64be | success | Build succeeded |
| snowpatch_ozlabs/build-ppc64e | success | Build succeeded |
| snowpatch_ozlabs/build-pmac32 | success | Build succeeded |
| snowpatch_ozlabs/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 18 lines checked |
| snowpatch_ozlabs/needsstable | warning | Please consider tagging this patch for stable! |
On 6/5/21 5:38 am, Christophe JAILLET wrote: > 'rc' is known to be 0 here. > Initialize 'rc' with the expected error code before using it. I would prefer: "In cxl_add_chardev(), if the call to device_create() fails, we print the error message before 'rc' is set correctly, and therefore always print 0. Move the error message after setting 'rc'." > > While at it, avoid the affectation of 'rc' in a 'if' to make things more > obvious and linux style. I strongly agree with this, some of the other cxl developers don't but they are wrong :) > > Fixes: f204e0b8ce ("cxl: Driver code for powernv PCIe based cards for userspace access") > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> Apart from my comment above: Acked-by: Andrew Donnellan <ajd@linux.ibm.com> Thanks for catching this!
On Wed, May 05, 2021 at 09:38:49PM +0200, Christophe JAILLET wrote: > 'rc' is known to be 0 here. > Initialize 'rc' with the expected error code before using it. > > While at it, avoid the affectation of 'rc' in a 'if' to make things more > obvious and linux style. > > Fixes: f204e0b8ce ("cxl: Driver code for powernv PCIe based cards for userspace access") You need a full 12 digits for the SHA1, otherwise our scripts complain about it :( I'll fix it up here, but please fix your tools. thanks, greg k-h
diff --git a/drivers/misc/cxl/file.c b/drivers/misc/cxl/file.c index bd3bd32333c5..3dbdce96fae0 100644 --- a/drivers/misc/cxl/file.c +++ b/drivers/misc/cxl/file.c @@ -569,7 +569,8 @@ static int cxl_add_chardev(struct cxl_afu *afu, dev_t devt, struct cdev *cdev, int rc; cdev_init(cdev, fops); - if ((rc = cdev_add(cdev, devt, 1))) { + rc = cdev_add(cdev, devt, 1); + if (rc) { dev_err(&afu->dev, "Unable to add %s chardev: %i\n", desc, rc); return rc; } @@ -577,8 +578,8 @@ static int cxl_add_chardev(struct cxl_afu *afu, dev_t devt, struct cdev *cdev, dev = device_create(cxl_class, &afu->dev, devt, afu, "afu%i.%i%s", afu->adapter->adapter_num, afu->slice, postfix); if (IS_ERR(dev)) { - dev_err(&afu->dev, "Unable to create %s chardev in sysfs: %i\n", desc, rc); rc = PTR_ERR(dev); + dev_err(&afu->dev, "Unable to create %s chardev in sysfs: %i\n", desc, rc); goto err; }
'rc' is known to be 0 here. Initialize 'rc' with the expected error code before using it. While at it, avoid the affectation of 'rc' in a 'if' to make things more obvious and linux style. Fixes: f204e0b8ce ("cxl: Driver code for powernv PCIe based cards for userspace access") Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> --- drivers/misc/cxl/file.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)