2017-07-26 13:09:01

by Himanshu Jha

Subject: [PATCH] Drivers : edac : checkpatch.pl clean up

Fixed 'no assignment in if condition' coding style issue and removed unnecessary spaces at the start of a line.

Signed-off-by: Himanshu Jha <[email protected]>
---
drivers/edac/i82860_edac.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/edac/i82860_edac.c b/drivers/edac/i82860_edac.c
index 236c813..c8c1c4d 100644
--- a/drivers/edac/i82860_edac.c
+++ b/drivers/edac/i82860_edac.c
@@ -282,7 +282,9 @@ static void i82860_remove_one(struct pci_dev *pdev)
if (i82860_pci)
edac_pci_release_generic_ctl(i82860_pci);

- if ((mci = edac_mc_del_mc(&pdev->dev)) == NULL)
+ mci = edac_mc_del_mc(&pdev->dev);
+
+ if (mci == NULL)
return;

edac_mc_free(mci);
@@ -312,10 +314,11 @@ static int __init i82860_init(void)

edac_dbg(3, "\n");

- /* Ensure that the OPSTATE is set correctly for POLL or NMI */
- opstate_init();
+ /* Ensure that the OPSTATE is set correctly for POLL or NMI */
+ opstate_init();

- if ((pci_rc = pci_register_driver(&i82860_driver)) < 0)
+ pci_rc = pci_register_driver(&i82860_driver);
+ if (pci_rc < 0)
goto fail0;

if (!mci_pdev) {
--
2.7.4

2017-07-26 14:32:57

by Borislav Petkov

Subject: Re: [PATCH] Drivers : edac : checkpatch.pl clean up

On Wed, Jul 26, 2017 at 06:37:48PM +0530, Himanshu Jha wrote:
> Fixed 'no assignment in if condition' coding style issue and removed unnecessary spaces at the start of a line.

Please put a shorter version of your commit message in the patch subject
- "[PATCH] Drivers : edac : checkpatch.pl clean up" is not very telling.

Ironically, if you run your patch through checkpatch, it will tell you so too:

WARNING: A patch subject line should describe the change not the tool that found it
#6:
Subject: [PATCH] Drivers : edac : checkpatch.pl clean up

which means, after you create a patch, run it through checkpatch too.

Also, the subject format should be:

"EDAC, i82860: ..."

Also, the tense in the commit message should not be past: "Fix 'no
assignment ... and remove ...".

> Signed-off-by: Himanshu Jha <[email protected]>
> ---
> drivers/edac/i82860_edac.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/edac/i82860_edac.c b/drivers/edac/i82860_edac.c
> index 236c813..c8c1c4d 100644
> --- a/drivers/edac/i82860_edac.c
> +++ b/drivers/edac/i82860_edac.c
> @@ -282,7 +282,9 @@ static void i82860_remove_one(struct pci_dev *pdev)
> if (i82860_pci)
> edac_pci_release_generic_ctl(i82860_pci);
>
> - if ((mci = edac_mc_del_mc(&pdev->dev)) == NULL)
> + mci = edac_mc_del_mc(&pdev->dev);
> +

No need for the newline.

There's an unnecessary newline in i82860_init_one() too, you can remove
that one too.

> + if (mci == NULL)

if (!mci)

> return;
>
> edac_mc_free(mci);
> @@ -312,10 +314,11 @@ static int __init i82860_init(void)
>
> edac_dbg(3, "\n");
>
> - /* Ensure that the OPSTATE is set correctly for POLL or NMI */
> - opstate_init();
> + /* Ensure that the OPSTATE is set correctly for POLL or NMI */
> + opstate_init();
>
> - if ((pci_rc = pci_register_driver(&i82860_driver)) < 0)
> + pci_rc = pci_register_driver(&i82860_driver);
> + if (pci_rc < 0)
> goto fail0;
>
> if (!mci_pdev) {
> --

Finally, do not linger too long fixing checkpatch issues but try to fix
some real bugs. Enough of those in the kernel :-)

Thanks.

--
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply.
--