Re: [PATCH 1/1] SCSI-QLA2XXX: Deletion of unnecessary checks before the function call "vfree"

Re: [PATCH 1/1] SCSI-QLA2XXX: Deletion of unnecessary checks before the function call "vfree"

[SF Markus Elfring]

>> If you are convinced that dropping the null tests is a good idea, then you 
>> can submit the patch that makes the change to the relevant maintainers and 
>> mailing lists.

>From ff44962f88ac2dae9324e30819629da4fb33f0ff Mon Sep 17 00:00:00 2001
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 22 Oct 2014 20:40:31 +0200
Subject: [PATCH] SCSI-QLA2XXX: Deletion of unnecessary checks before the
 function call "vfree"

A semantic patch approach was proposed with the subject "[PATCH with
Coccinelle?] Deletion of unnecessary checks before specific function calls"
on 2014-03-05.
https://lkml.org/lkml/2014/3/5/344
http://article.gmane.org/gmane.comp.version-control.coccinelle/3513/

This patch pattern application was repeated with the help of the software
"Coccinelle 1.0.0-rc22" on the source files for Linux 3.17.1. An extract
of the automatically generated update suggestions is shown here.

It was determined that the affected source code places call functions
which perform input parameter validation already. It is therefore not
needed that a similar safety check is repeated at the call site.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/scsi/qla2xxx/qla_attr.c |  6 ++----
 drivers/scsi/qla2xxx/qla_init.c | 18 ++++++------------
 drivers/scsi/qla2xxx/qla_os.c   |  6 ++----
 3 files changed, 10 insertions(+), 20 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_attr.c b/drivers/scsi/qla2xxx/qla_attr.c
index 82b92c4..95c4c09 100644
--- a/drivers/scsi/qla2xxx/qla_attr.c
+++ b/drivers/scsi/qla2xxx/qla_attr.c
@@ -175,10 +175,8 @@ qla2x00_sysfs_write_fw_dump_template(struct file *filp,
struct kobject *kobj,
 	uint32_t size;

 	if (off == 0) {
-		if (ha->fw_dump)
-			vfree(ha->fw_dump);
-		if (ha->fw_dump_template)
-			vfree(ha->fw_dump_template);
+		vfree(ha->fw_dump);
+		vfree(ha->fw_dump_template);

 		ha->fw_dump = NULL;
 		ha->fw_dump_len = 0;
diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index a4dde7e..8da3d4f 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -5256,8 +5256,7 @@ qla24xx_load_risc_flash(scsi_qla_host_t *vha, uint32_t
*srisc_addr,
 	if (!IS_QLA27XX(ha))
 		return rval;

-	if (ha->fw_dump_template)
-		vfree(ha->fw_dump_template);
+	vfree(ha->fw_dump_template);
 	ha->fw_dump_template = NULL;
 	ha->fw_dump_template_len = 0;

@@ -5307,8 +5306,7 @@ qla24xx_load_risc_flash(scsi_qla_host_t *vha, uint32_t
*srisc_addr,

 default_template:
 	ql_log(ql_log_warn, vha, 0x0168, "Using default fwdump template\n");
-	if (ha->fw_dump_template)
-		vfree(ha->fw_dump_template);
+	vfree(ha->fw_dump_template);
 	ha->fw_dump_template = NULL;
 	ha->fw_dump_template_len = 0;

@@ -5342,8 +5340,7 @@ default_template:

 failed_template:
 	ql_log(ql_log_warn, vha, 0x016d, "Failed default fwdump template\n");
-	if (ha->fw_dump_template)
-		vfree(ha->fw_dump_template);
+	vfree(ha->fw_dump_template);
 	ha->fw_dump_template = NULL;
 	ha->fw_dump_template_len = 0;
 	return rval;
@@ -5559,8 +5556,7 @@ qla24xx_load_risc_blob(scsi_qla_host_t *vha, uint32_t
*srisc_addr)
 	if (!IS_QLA27XX(ha))
 		return rval;

-	if (ha->fw_dump_template)
-		vfree(ha->fw_dump_template);
+	vfree(ha->fw_dump_template);
 	ha->fw_dump_template = NULL;
 	ha->fw_dump_template_len = 0;

@@ -5609,8 +5605,7 @@ qla24xx_load_risc_blob(scsi_qla_host_t *vha, uint32_t
*srisc_addr)

 default_template:
 	ql_log(ql_log_warn, vha, 0x0178, "Using default fwdump template\n");
-	if (ha->fw_dump_template)
-		vfree(ha->fw_dump_template);
+	vfree(ha->fw_dump_template);
 	ha->fw_dump_template = NULL;
 	ha->fw_dump_template_len = 0;

@@ -5644,8 +5639,7 @@ default_template:

 failed_template:
 	ql_log(ql_log_warn, vha, 0x017d, "Failed default fwdump template\n");
-	if (ha->fw_dump_template)
-		vfree(ha->fw_dump_template);
+	vfree(ha->fw_dump_template);
 	ha->fw_dump_template = NULL;
 	ha->fw_dump_template_len = 0;
 	return rval;
diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index db3dbd9..0f9c378 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -3676,10 +3676,8 @@ qla2x00_free_fw_dump(struct qla_hw_data *ha)
 		dma_free_coherent(&ha->pdev->dev,
 		    EFT_SIZE, ha->eft, ha->eft_dma);

-	if (ha->fw_dump)
-		vfree(ha->fw_dump);
-	if (ha->fw_dump_template)
-		vfree(ha->fw_dump_template);
+	vfree(ha->fw_dump);
+	vfree(ha->fw_dump_template);

 	ha->fce = NULL;
 	ha->fce_dma = 0;
-- 
2.1.2

Re: [PATCH 1/1] SCSI-QLA2...: Deletion of unnecessary checks before the function call "vfree"

[SF Markus Elfring]

>> If you are convinced that dropping the null tests is a good idea, then you 
>> can submit the patch that makes the change to the relevant maintainers and 
>> mailing lists.

I resent the request once more because another "Triple-X" software development
adventure might follow ...?

Regards,
Markus


>From ff44962f88ac2dae9324e30819629da4fb33f0ff Mon Sep 17 00:00:00 2001
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 22 Oct 2014 20:40:31 +0200
Subject: [PATCH] SCSI-QLA2XXX: Deletion of unnecessary checks before the
 function call "vfree"

A semantic patch approach was proposed with the subject "[PATCH with
Coccinelle?] Deletion of unnecessary checks before specific function calls"
on 2014-03-05.
https://lkml.org/lkml/2014/3/5/344
http://article.gmane.org/gmane.comp.version-control.coccinelle/3513/

This patch pattern application was repeated with the help of the software
"Coccinelle 1.0.0-rc22" on the source files for Linux 3.17.1. An extract
of the automatically generated update suggestions is shown here.

It was determined that the affected source code places call functions
which perform input parameter validation already. It is therefore not
needed that a similar safety check is repeated at the call site.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/scsi/qla2xxx/qla_attr.c |  6 ++----
 drivers/scsi/qla2xxx/qla_init.c | 18 ++++++------------
 drivers/scsi/qla2xxx/qla_os.c   |  6 ++----
 3 files changed, 10 insertions(+), 20 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_attr.c b/drivers/scsi/qla2xxx/qla_attr.c
index 82b92c4..95c4c09 100644
--- a/drivers/scsi/qla2xxx/qla_attr.c
+++ b/drivers/scsi/qla2xxx/qla_attr.c
@@ -175,10 +175,8 @@ qla2x00_sysfs_write_fw_dump_template(struct file *filp,
struct kobject *kobj,
 	uint32_t size;

 	if (off == 0) {
-		if (ha->fw_dump)
-			vfree(ha->fw_dump);
-		if (ha->fw_dump_template)
-			vfree(ha->fw_dump_template);
+		vfree(ha->fw_dump);
+		vfree(ha->fw_dump_template);

 		ha->fw_dump = NULL;
 		ha->fw_dump_len = 0;
diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index a4dde7e..8da3d4f 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -5256,8 +5256,7 @@ qla24xx_load_risc_flash(scsi_qla_host_t *vha, uint32_t
*srisc_addr,
 	if (!IS_QLA27XX(ha))
 		return rval;

-	if (ha->fw_dump_template)
-		vfree(ha->fw_dump_template);
+	vfree(ha->fw_dump_template);
 	ha->fw_dump_template = NULL;
 	ha->fw_dump_template_len = 0;

@@ -5307,8 +5306,7 @@ qla24xx_load_risc_flash(scsi_qla_host_t *vha, uint32_t
*srisc_addr,

 default_template:
 	ql_log(ql_log_warn, vha, 0x0168, "Using default fwdump template\n");
-	if (ha->fw_dump_template)
-		vfree(ha->fw_dump_template);
+	vfree(ha->fw_dump_template);
 	ha->fw_dump_template = NULL;
 	ha->fw_dump_template_len = 0;

@@ -5342,8 +5340,7 @@ default_template:

 failed_template:
 	ql_log(ql_log_warn, vha, 0x016d, "Failed default fwdump template\n");
-	if (ha->fw_dump_template)
-		vfree(ha->fw_dump_template);
+	vfree(ha->fw_dump_template);
 	ha->fw_dump_template = NULL;
 	ha->fw_dump_template_len = 0;
 	return rval;
@@ -5559,8 +5556,7 @@ qla24xx_load_risc_blob(scsi_qla_host_t *vha, uint32_t
*srisc_addr)
 	if (!IS_QLA27XX(ha))
 		return rval;

-	if (ha->fw_dump_template)
-		vfree(ha->fw_dump_template);
+	vfree(ha->fw_dump_template);
 	ha->fw_dump_template = NULL;
 	ha->fw_dump_template_len = 0;

@@ -5609,8 +5605,7 @@ qla24xx_load_risc_blob(scsi_qla_host_t *vha, uint32_t
*srisc_addr)

 default_template:
 	ql_log(ql_log_warn, vha, 0x0178, "Using default fwdump template\n");
-	if (ha->fw_dump_template)
-		vfree(ha->fw_dump_template);
+	vfree(ha->fw_dump_template);
 	ha->fw_dump_template = NULL;
 	ha->fw_dump_template_len = 0;

@@ -5644,8 +5639,7 @@ default_template:

 failed_template:
 	ql_log(ql_log_warn, vha, 0x017d, "Failed default fwdump template\n");
-	if (ha->fw_dump_template)
-		vfree(ha->fw_dump_template);
+	vfree(ha->fw_dump_template);
 	ha->fw_dump_template = NULL;
 	ha->fw_dump_template_len = 0;
 	return rval;
diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index db3dbd9..0f9c378 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -3676,10 +3676,8 @@ qla2x00_free_fw_dump(struct qla_hw_data *ha)
 		dma_free_coherent(&ha->pdev->dev,
 		    EFT_SIZE, ha->eft, ha->eft_dma);

-	if (ha->fw_dump)
-		vfree(ha->fw_dump);
-	if (ha->fw_dump_template)
-		vfree(ha->fw_dump_template);
+	vfree(ha->fw_dump);
+	vfree(ha->fw_dump_template);

 	ha->fce = NULL;
 	ha->fce_dma = 0;
-- 
2.1.2

[PATCH 1/1] SCSI-OSD: Deletion of an unnecessary check before the function call "put_disk"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 20 Nov 2014 18:11:28 +0100

The put_disk() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/scsi/osd/osd_uld.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/scsi/osd/osd_uld.c b/drivers/scsi/osd/osd_uld.c
index e1d9a4c..3d82ee1 100644
--- a/drivers/scsi/osd/osd_uld.c
+++ b/drivers/scsi/osd/osd_uld.c
@@ -408,8 +408,7 @@ static void __remove(struct device *dev)
 	OSD_INFO("osd_remove %s\n",
 		 oud->disk ? oud->disk->disk_name : NULL);
 
-	if (oud->disk)
-		put_disk(oud->disk);
+	put_disk(oud->disk);
 	ida_remove(&osd_minor_ida, oud->minor);
 
 	kfree(oud);
-- 
2.1.3

[PATCH 1/1] SCSI: Deletion of unnecessary checks before the function call "put_device"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 20 Nov 2014 18:48:52 +0100

The put_device() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/scsi/hosts.c      | 3 +--
 drivers/scsi/scsi_sysfs.c | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index 6de80e3..57afb19 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -337,8 +337,7 @@ static void scsi_host_dev_release(struct device *dev)
 
 	kfree(shost->shost_data);
 
-	if (parent)
-		put_device(parent);
+	put_device(parent);
 	kfree(shost);
 }
 
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 8b4105a..c977c99 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -426,8 +426,7 @@ static void scsi_device_dev_release_usercontext(struct work_struct *work)
 	kfree(sdev->inquiry);
 	kfree(sdev);
 
-	if (parent)
-		put_device(parent);
+	put_device(parent);
 }
 
 static void scsi_device_dev_release(struct device *dev)
-- 
2.1.3

[PATCH 1/1] SCSI-libfc: Deletion of an unnecessary check before the function call "fc_fcp_ddp_done"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 20 Nov 2014 20:10:19 +0100

The fc_fcp_ddp_done() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/scsi/libfc/fc_exch.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c
index 1b3a094..aad6f48 100644
--- a/drivers/scsi/libfc/fc_exch.c
+++ b/drivers/scsi/libfc/fc_exch.c
@@ -2120,8 +2120,7 @@ static struct fc_seq *fc_exch_seq_send(struct fc_lport *lport,
 	spin_unlock_bh(&ep->ex_lock);
 	return sp;
 err:
-	if (fsp)
-		fc_fcp_ddp_done(fsp);
+	fc_fcp_ddp_done(fsp);
 	rc = fc_exch_done_locked(ep);
 	spin_unlock_bh(&ep->ex_lock);
 	if (!rc)
-- 
2.1.3

[PATCH 1/1] SCSI-eata_pio: Deletion of an unnecessary check before the function call "pci_dev_put"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 20 Nov 2014 20:37:30 +0100

The pci_dev_put() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/scsi/eata_pio.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/scsi/eata_pio.c b/drivers/scsi/eata_pio.c
index 8319d2b..707f64d 100644
--- a/drivers/scsi/eata_pio.c
+++ b/drivers/scsi/eata_pio.c
@@ -122,8 +122,7 @@ static int eata_pio_release(struct Scsi_Host *sh)
 			release_region(sh->io_port, sh->n_io_port);
 	}
 	/* At this point the PCI reference can go */
-	if (hd->pdev)
-		pci_dev_put(hd->pdev);
+	pci_dev_put(hd->pdev);
 	return 1;
 }
 
-- 
2.1.3

[PATCH 1/1] SCSI-aic94xx: Deletion of an unnecessary check before the function call "kfree"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 20 Nov 2014 23:18:56 +0100

The kfree() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/scsi/aic94xx/aic94xx_init.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/scsi/aic94xx/aic94xx_init.c b/drivers/scsi/aic94xx/aic94xx_init.c
index c56741f..0bfdcba 100644
--- a/drivers/scsi/aic94xx/aic94xx_init.c
+++ b/drivers/scsi/aic94xx/aic94xx_init.c
@@ -597,8 +597,7 @@ static void asd_destroy_ha_caches(struct asd_ha_struct *asd_ha)
 	if (asd_ha->hw_prof.scb_ext)
 		asd_free_coherent(asd_ha, asd_ha->hw_prof.scb_ext);
 
-	if (asd_ha->hw_prof.ddb_bitmap)
-		kfree(asd_ha->hw_prof.ddb_bitmap);
+	kfree(asd_ha->hw_prof.ddb_bitmap);
 	asd_ha->hw_prof.ddb_bitmap = NULL;
 
 	for (i = 0; i < ASD_MAX_PHYS; i++) {
-- 
2.1.3

[PATCH 1/1] SCSI-bfa: Deletion of an unnecessary check before the function call "vfree"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 20 Nov 2014 23:43:17 +0100

The vfree() function performs also input parameter validation. Thus the test
around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/scsi/bfa/bfad_debugfs.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/scsi/bfa/bfad_debugfs.c b/drivers/scsi/bfa/bfad_debugfs.c
index 8e83d04..a72170f 100644
--- a/drivers/scsi/bfa/bfad_debugfs.c
+++ b/drivers/scsi/bfa/bfad_debugfs.c
@@ -399,8 +399,7 @@ bfad_debugfs_release_fwtrc(struct inode *inode, struct file *file)
 	if (!fw_debug)
 		return 0;
 
-	if (fw_debug->debug_buffer)
-		vfree(fw_debug->debug_buffer);
+	vfree(fw_debug->debug_buffer);
 
 	file->private_data = NULL;
 	kfree(fw_debug);
-- 
2.1.3

RE: [PATCH 1/1] SCSI-bfa: Deletion of an unnecessary check before the function call "vfree"

[Anil Gurumurthy]

[-- Attachment #1: Type: text/plain, Size: 1355 bytes --]

Patch looks good. 
Thanks!
Acked-by: Anil Gurumurthy <anil.gurumurthy@qlogic.com>

-----Original Message-----
From: SF Markus Elfring [mailto:elfring@users.sourceforge.net] 
Sent: 21 November 2014 04:17
To: Anil Gurumurthy; James E. J. Bottomley; Sudarsana Kalluru; linux-scsi
Cc: linux-kernel; kernel-janitors@vger.kernel.org; Julia Lawall
Subject: [PATCH 1/1] SCSI-bfa: Deletion of an unnecessary check before the function call "vfree"

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 20 Nov 2014 23:43:17 +0100

The vfree() function performs also input parameter validation. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/scsi/bfa/bfad_debugfs.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/scsi/bfa/bfad_debugfs.c b/drivers/scsi/bfa/bfad_debugfs.c index 8e83d04..a72170f 100644
--- a/drivers/scsi/bfa/bfad_debugfs.c
+++ b/drivers/scsi/bfa/bfad_debugfs.c
@@ -399,8 +399,7 @@ bfad_debugfs_release_fwtrc(struct inode *inode, struct file *file)
 	if (!fw_debug)
 		return 0;
 
-	if (fw_debug->debug_buffer)
-		vfree(fw_debug->debug_buffer);
+	vfree(fw_debug->debug_buffer);
 
 	file->private_data = NULL;
 	kfree(fw_debug);
--
2.1.3


[-- Attachment #2: winmail.dat --]
[-- Type: application/ms-tnef, Size: 6080 bytes --]

[PATCH 1/1] SCSI-libcxgbi: Deletion of an unnecessary check before the function call "dst_release"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 21 Nov 2014 09:15:10 +0100

The dst_release() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/scsi/cxgbi/libcxgbi.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/scsi/cxgbi/libcxgbi.c b/drivers/scsi/cxgbi/libcxgbi.c
index addd1dd..95d2654 100644
--- a/drivers/scsi/cxgbi/libcxgbi.c
+++ b/drivers/scsi/cxgbi/libcxgbi.c
@@ -798,8 +798,7 @@ void cxgbi_sock_closed(struct cxgbi_sock *csk)
 		return;
 	if (csk->saddr.sin_port)
 		sock_put_port(csk);
-	if (csk->dst)
-		dst_release(csk->dst);
+	dst_release(csk->dst);
 	csk->cdev->csk_release_offload_resources(csk);
 	cxgbi_sock_set_state(csk, CTP_CLOSED);
 	cxgbi_inform_iscsi_conn_closing(csk);
-- 
2.1.3

[PATCH 1/1] SCSI-fnic: Deletion of an unnecessary check before the function call "vfree"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 21 Nov 2014 09:39:43 +0100

The vfree() function performs also input parameter validation. Thus the test
around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/scsi/fnic/fnic_debugfs.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/scsi/fnic/fnic_debugfs.c b/drivers/scsi/fnic/fnic_debugfs.c
index 2c613bd..ec70e6d 100644
--- a/drivers/scsi/fnic/fnic_debugfs.c
+++ b/drivers/scsi/fnic/fnic_debugfs.c
@@ -102,8 +102,7 @@ void fnic_debugfs_terminate(void)
 	debugfs_remove(fnic_trace_debugfs_root);
 	fnic_trace_debugfs_root = NULL;
 
-	if (fc_trc_flag)
-		vfree(fc_trc_flag);
+	vfree(fc_trc_flag);
 }
 
 /*
-- 
2.1.3

[PATCH 1/1] target: Deletion of unnecessary checks before the function call "module_put"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 21 Nov 2014 10:25:45 +0100

The module_put() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/target/iscsi/iscsi_target_transport.c | 3 +--
 drivers/target/target_core_hba.c              | 6 ++----
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target_transport.c b/drivers/target/iscsi/iscsi_target_transport.c
index 882728f..08217d6 100644
--- a/drivers/target/iscsi/iscsi_target_transport.c
+++ b/drivers/target/iscsi/iscsi_target_transport.c
@@ -26,8 +26,7 @@ struct iscsit_transport *iscsit_get_transport(int type)
 
 void iscsit_put_transport(struct iscsit_transport *t)
 {
-	if (t->owner)
-		module_put(t->owner);
+	module_put(t->owner);
 }
 
 int iscsit_register_transport(struct iscsit_transport *t)
diff --git a/drivers/target/target_core_hba.c b/drivers/target/target_core_hba.c
index a25051a..e6e496f 100644
--- a/drivers/target/target_core_hba.c
+++ b/drivers/target/target_core_hba.c
@@ -137,8 +137,7 @@ core_alloc_hba(const char *plugin_name, u32 plugin_dep_id, u32 hba_flags)
 	return hba;
 
 out_module_put:
-	if (hba->transport->owner)
-		module_put(hba->transport->owner);
+	module_put(hba->transport->owner);
 	hba->transport = NULL;
 out_free_hba:
 	kfree(hba);
@@ -159,8 +158,7 @@ core_delete_hba(struct se_hba *hba)
 	pr_debug("CORE_HBA[%d] - Detached HBA from Generic Target"
 			" Core\n", hba->hba_id);
 
-	if (hba->transport->owner)
-		module_put(hba->transport->owner);
+	module_put(hba->transport->owner);
 
 	hba->transport = NULL;
 	kfree(hba);
-- 
2.1.3

Re: [PATCH 1/1] target: Deletion of unnecessary checks before the function call "module_put"

[Nicholas A. Bellinger]

On Fri, 2014-11-21 at 10:30 +0100, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Fri, 21 Nov 2014 10:25:45 +0100
> 
> The module_put() function tests whether its argument is NULL and then
> returns immediately. Thus the test around the call is not needed.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/target/iscsi/iscsi_target_transport.c | 3 +--
>  drivers/target/target_core_hba.c              | 6 ++----
>  2 files changed, 3 insertions(+), 6 deletions(-)
> 

Applied to target-pending/for-next.

Thanks Markus!

--nab

[PATCH 0/3] block: Deletion of checks before three function calls

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 18 Jan 2015 14:50:06 +0100

Functions which release a system resource are occasionally documented in the way
that they tolerate the passing of a null pointer for example. I do not see a need
because of this fact that a function caller repeats a corresponding check.

Markus Elfring (3):
  cciss: Deletion of an unnecessary check before the function call "put_disk"
  z2ram: Delete an unnecessary check before the function call "kfree"
  Deletion of checks before the function call "iounmap"

 drivers/block/cciss.c    | 18 ++++++------------
 drivers/block/mg_disk.c  |  3 +--
 drivers/block/skd_main.c |  6 ++----
 3 files changed, 9 insertions(+), 18 deletions(-)

-- 
2.2.2

[PATCH 1/3] block-cciss: Deletion of an unnecessary check before the function call "put_disk"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 18 Jan 2015 11:14:16 +0100

The put_disk() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/block/cciss.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index ff20f19..4be0de0 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -4424,8 +4424,7 @@ static void free_hba(ctlr_info_t *h)
 
 	hba[h->ctlr] = NULL;
 	for (i = 0; i < h->highest_lun + 1; i++)
-		if (h->gendisk[i] != NULL)
-			put_disk(h->gendisk[i]);
+		put_disk(h->gendisk[i]);
 	kfree(h);
 }
 
-- 
2.2.2

[PATCH 3/3] block: Deletion of checks before the function call "iounmap"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 18 Jan 2015 13:33:25 +0100

The iounmap() function performs also input parameter validation.
Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/block/cciss.c    | 18 ++++++------------
 drivers/block/mg_disk.c  |  3 +--
 drivers/block/skd_main.c |  6 ++----
 3 files changed, 9 insertions(+), 18 deletions(-)

diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index ff20f19..d13ef0f 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -4382,12 +4382,9 @@ err_out_free_res:
 	 * Deliberately omit pci_disable_device(): it does something nasty to
 	 * Smart Array controllers that pci_enable_device does not undo
 	 */
-	if (h->transtable)
-		iounmap(h->transtable);
-	if (h->cfgtable)
-		iounmap(h->cfgtable);
-	if (h->vaddr)
-		iounmap(h->vaddr);
+	iounmap(h->transtable);
+	iounmap(h->cfgtable);
+	iounmap(h->vaddr);
 	pci_release_regions(h->pdev);
 	return err;
 }
@@ -4939,12 +4936,9 @@ static void cciss_undo_allocations_after_kdump_soft_reset(ctlr_info_t *h)
 	if (h->reply_pool)
 		pci_free_consistent(h->pdev, h->max_commands * sizeof(__u64),
 				h->reply_pool, h->reply_pool_dhandle);
-	if (h->transtable)
-		iounmap(h->transtable);
-	if (h->cfgtable)
-		iounmap(h->cfgtable);
-	if (h->vaddr)
-		iounmap(h->vaddr);
+	iounmap(h->transtable);
+	iounmap(h->cfgtable);
+	iounmap(h->vaddr);
 	unregister_blkdev(h->major, h->devname);
 	cciss_destroy_hba_sysfs_entry(h);
 	pci_release_regions(h->pdev);
diff --git a/drivers/block/mg_disk.c b/drivers/block/mg_disk.c
index 145ce2a..9f738ee 100644
--- a/drivers/block/mg_disk.c
+++ b/drivers/block/mg_disk.c
@@ -1068,8 +1068,7 @@ static int mg_remove(struct platform_device *plat_dev)
 		gpio_free(host->rst);
 
 	/* unmap io */
-	if (host->dev_base)
-		iounmap(host->dev_base);
+	iounmap(host->dev_base);
 
 	/* free mg_host */
 	kfree(host);
diff --git a/drivers/block/skd_main.c b/drivers/block/skd_main.c
index 1e46eb2..d4c7c4b 100644
--- a/drivers/block/skd_main.c
+++ b/drivers/block/skd_main.c
@@ -4944,8 +4944,7 @@ err_out_timer:
 
 err_out_iounmap:
 	for (i = 0; i < SKD_MAX_BARS; i++)
-		if (skdev->mem_map[i])
-			iounmap(skdev->mem_map[i]);
+		iounmap(skdev->mem_map[i]);
 
 	if (skdev->pcie_error_reporting_is_enabled)
 		pci_disable_pcie_error_reporting(pdev);
@@ -5108,8 +5107,7 @@ err_out_timer:
 
 err_out_iounmap:
 	for (i = 0; i < SKD_MAX_BARS; i++)
-		if (skdev->mem_map[i])
-			iounmap(skdev->mem_map[i]);
+		iounmap(skdev->mem_map[i]);
 
 	if (skdev->pcie_error_reporting_is_enabled)
 		pci_disable_pcie_error_reporting(pdev);
-- 
2.2.2

[PATCH] SCSI-bfa: Delete more unnecessary checks before the function call "vfree"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 5 Feb 2015 22:02:16 +0100

The vfree() function performs also input parameter validation.
Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/scsi/bfa/bfad.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/bfa/bfad.c b/drivers/scsi/bfa/bfad.c
index e90a374..05c8ca0 100644
--- a/drivers/scsi/bfa/bfad.c
+++ b/drivers/scsi/bfa/bfad.c
@@ -1810,11 +1810,11 @@ bfad_load_fwimg(struct pci_dev *pdev)
 static void
 bfad_free_fwimg(void)
 {
-	if (bfi_image_ct2_size && bfi_image_ct2)
+	if (bfi_image_ct2_size)
 		vfree(bfi_image_ct2);
-	if (bfi_image_ct_size && bfi_image_ct)
+	if (bfi_image_ct_size)
 		vfree(bfi_image_ct);
-	if (bfi_image_cb_size && bfi_image_cb)
+	if (bfi_image_cb_size)
 		vfree(bfi_image_cb);
 }
 
-- 
2.2.2

[PATCH] SCSI-aic7...: Delete unnecessary checks before the function call "kfree"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 5 Feb 2015 22:23:48 +0100

The kfree() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/scsi/aic7xxx/aic7xxx_core.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/scsi/aic7xxx/aic7xxx_core.c b/drivers/scsi/aic7xxx/aic7xxx_core.c
index 10172a3..98a5ed1 100644
--- a/drivers/scsi/aic7xxx/aic7xxx_core.c
+++ b/drivers/scsi/aic7xxx/aic7xxx_core.c
@@ -2200,8 +2200,7 @@ ahc_free_tstate(struct ahc_softc *ahc, u_int scsi_id, char channel, int force)
 	if (channel == 'B')
 		scsi_id += 8;
 	tstate = ahc->enabled_targets[scsi_id];
-	if (tstate != NULL)
-		kfree(tstate);
+	kfree(tstate);
 	ahc->enabled_targets[scsi_id] = NULL;
 }
 #endif
@@ -4482,8 +4481,7 @@ ahc_set_unit(struct ahc_softc *ahc, int unit)
 void
 ahc_set_name(struct ahc_softc *ahc, char *name)
 {
-	if (ahc->name != NULL)
-		kfree(ahc->name);
+	kfree(ahc->name);
 	ahc->name = name;
 }
 
@@ -4550,10 +4548,8 @@ ahc_free(struct ahc_softc *ahc)
 		kfree(ahc->black_hole);
 	}
 #endif
-	if (ahc->name != NULL)
-		kfree(ahc->name);
-	if (ahc->seep_config != NULL)
-		kfree(ahc->seep_config);
+	kfree(ahc->name);
+	kfree(ahc->seep_config);
 #ifndef __FreeBSD__
 	kfree(ahc);
 #endif
@@ -4958,8 +4954,7 @@ ahc_fini_scbdata(struct ahc_softc *ahc)
 	case 0:
 		break;
 	}
-	if (scb_data->scbarray != NULL)
-		kfree(scb_data->scbarray);
+	kfree(scb_data->scbarray);
 }
 
 static void
-- 
2.2.2

[PATCH 0/2] SCSI-lpfc: Deletion of an unnecessary check

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 5 Feb 2015 23:17:52 +0100

Another update suggestion was taken into account after a patch was applied
from static source code analysis.

Markus Elfring (2):
  Delete an unnecessary check before the function call "kfree"
  One function call less in lpfc_bsg_hba_set_event() after error detection

 drivers/scsi/lpfc/lpfc_bsg.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

-- 
2.2.2

[PATCH 1/2] SCSI-lpfc: Delete an unnecessary check before the function call "kfree"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 5 Feb 2015 22:45:02 +0100

The kfree() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/scsi/lpfc/lpfc_bsg.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_bsg.c b/drivers/scsi/lpfc/lpfc_bsg.c
index a7bf359..1af783a 100644
--- a/drivers/scsi/lpfc/lpfc_bsg.c
+++ b/drivers/scsi/lpfc/lpfc_bsg.c
@@ -1261,9 +1261,7 @@ lpfc_bsg_hba_set_event(struct fc_bsg_job *job)
 	return 0; /* call job done later */
 
 job_error:
-	if (dd_data != NULL)
-		kfree(dd_data);
-
+	kfree(dd_data);
 	job->dd_data = NULL;
 	return rc;
 }
-- 
2.2.2

[PATCH 2/2] SCSI-lpfc: One function call less in lpfc_bsg_hba_set_event() after error detection

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 5 Feb 2015 23:03:52 +0100

The kfree() function was called in two cases by the lpfc_bsg_hba_set_event()
function during error handling even if the passed variable "dd_data" contained
still a null pointer.

This implementation detail could be improved by the introduction of another
jump label.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/scsi/lpfc/lpfc_bsg.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_bsg.c b/drivers/scsi/lpfc/lpfc_bsg.c
index 1af783a..b8074cf 100644
--- a/drivers/scsi/lpfc/lpfc_bsg.c
+++ b/drivers/scsi/lpfc/lpfc_bsg.c
@@ -1240,7 +1240,7 @@ lpfc_bsg_hba_set_event(struct fc_bsg_job *job)
 					"2617 Failed allocation of event "
 					"waiter\n");
 			rc = -ENOMEM;
-			goto job_error;
+			goto free_data;
 		}
 		dd_data->type = TYPE_EVT;
 		dd_data->set_job = NULL;
@@ -1260,8 +1260,9 @@ lpfc_bsg_hba_set_event(struct fc_bsg_job *job)
 	spin_unlock_irqrestore(&phba->ct_ev_lock, flags);
 	return 0; /* call job done later */
 
-job_error:
+free_data:
 	kfree(dd_data);
+job_error:
 	job->dd_data = NULL;
 	return rc;
 }
-- 
2.2.2

[PATCH 0/3] SCSI-debug: Deletion of a few unnecessary checks

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 6 Feb 2015 18:08:53 +0100

Further update suggestions were taken into account after a patch was applied
from static source code analysis.

Markus Elfring (3):
  Deletion of unnecessary checks before the function call "vfree"
  Less function calls in scsi_debug_init() after error detection
  Fix exception handling for an alignment/granularity mismatch
    in scsi_debug_init()

 drivers/scsi/scsi_debug.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

-- 
2.2.2

[PATCH 1/3] SCSI-debug: Deletion of unnecessary checks before the function call "vfree"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 6 Feb 2015 16:56:57 +0100

The vfree() function performs also input parameter validation.
Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/scsi/scsi_debug.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 1132321..9e4add7 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -4969,12 +4969,9 @@ bus_unreg:
 dev_unreg:
 	root_device_unregister(pseudo_primary);
 free_vm:
-	if (map_storep)
-		vfree(map_storep);
-	if (dif_storep)
-		vfree(dif_storep);
+	vfree(map_storep);
+	vfree(dif_storep);
 	vfree(fake_storep);
-
 	return ret;
 }
 
@@ -4989,10 +4986,7 @@ static void __exit scsi_debug_exit(void)
 	driver_unregister(&sdebug_driverfs_driver);
 	bus_unregister(&pseudo_lld_bus);
 	root_device_unregister(pseudo_primary);
-
-	if (dif_storep)
-		vfree(dif_storep);
-
+	vfree(dif_storep);
 	vfree(fake_storep);
 }
 
-- 
2.2.2

[PATCH 2/3] SCSI-debug: Less function calls in scsi_debug_init() after error detection

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 6 Feb 2015 17:48:34 +0100

The vfree() function was called in two cases by the scsi_debug_init() function
during error handling even if the passed variables "dif_storep" and
"map_storep" contained null pointers eventually.

This implementation detail could be improved by the introduction of two
jump labels.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/scsi/scsi_debug.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 9e4add7..756b7be 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -4887,7 +4887,7 @@ static int __init scsi_debug_init(void)
 		if (dif_storep == NULL) {
 			pr_err("%s: out of mem. (DIX)\n", __func__);
 			ret = -ENOMEM;
-			goto free_vm;
+			goto free_fake;
 		}
 
 		memset(dif_storep, 0xff, dif_size);
@@ -4920,7 +4920,7 @@ static int __init scsi_debug_init(void)
 		if (map_storep == NULL) {
 			pr_err("%s: out of mem. (MAP)\n", __func__);
 			ret = -ENOMEM;
-			goto free_vm;
+			goto free_dif;
 		}
 
 		bitmap_zero(map_storep, map_size);
@@ -4934,7 +4934,7 @@ static int __init scsi_debug_init(void)
 	if (IS_ERR(pseudo_primary)) {
 		pr_warn("%s: root_device_register() error\n", __func__);
 		ret = PTR_ERR(pseudo_primary);
-		goto free_vm;
+		goto free_map;
 	}
 	ret = bus_register(&pseudo_lld_bus);
 	if (ret < 0) {
@@ -4968,9 +4968,11 @@ bus_unreg:
 	bus_unregister(&pseudo_lld_bus);
 dev_unreg:
 	root_device_unregister(pseudo_primary);
-free_vm:
+free_map:
 	vfree(map_storep);
+free_dif:
 	vfree(dif_storep);
+free_fake:
 	vfree(fake_storep);
 	return ret;
 }
-- 
2.2.2

[PATCH 3/3] SCSI-debug: Fix exception handling for an alignment/granularity mismatch in scsi_debug_init()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 6 Feb 2015 17:57:32 +0100

The scsi_debug_init() function returned directly if the data items "alignment"
and "granularity" did not fit together.

Let us improve the affected exception handling by replacing the return
statement by a goto statement so that previously allocated resources will also
be appropriately released.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/scsi/scsi_debug.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 756b7be..2c78663 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -4909,7 +4909,8 @@ static int __init scsi_debug_init(void)
 		    scsi_debug_unmap_alignment) {
 			pr_err("%s: ERR: unmap_granularity <= unmap_alignment\n",
 			       __func__);
-			return -EINVAL;
+			ret = -EINVAL;
+			goto free_dif;
 		}
 
 		map_size = lba_to_map_index(sdebug_store_sectors - 1) + 1;
-- 
2.2.2

[PATCH] SCSI-csiostor: Delete an unnecessary check before the function call "release_firmware"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 6 Feb 2015 19:10:25 +0100

The release_firmware() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/scsi/csiostor/csio_hw.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/scsi/csiostor/csio_hw.c b/drivers/scsi/csiostor/csio_hw.c
index 35c5f83..37fa8bf 100644
--- a/drivers/scsi/csiostor/csio_hw.c
+++ b/drivers/scsi/csiostor/csio_hw.c
@@ -2117,8 +2117,7 @@ csio_hw_flash_fw(struct csio_hw *hw, int *reset)
 			 hw->fw_state, reset);
 
 	/* Cleaning up */
-	if (fw != NULL)
-		release_firmware(fw);
+	release_firmware(fw);
 	kfree(card_fw);
 	return ret;
 }
-- 
2.2.2

[PATCH 0/4] SCSI-QLA4...: Deletion of some unnecessary checks

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 6 Feb 2015 23:03:41 +0100

Further update suggestions were taken into account after a patch was applied
from static source code analysis.

Markus Elfring (4):
  Deletion of unnecessary checks before three function calls
  Less function calls in qla4xxx_is_session_exists() after error detection
  Less function calls in qla4xxx_is_flash_ddb_exists() after error detection
  Less function calls in qla4xxx_sysfs_ddb_logout() after error detection

 drivers/scsi/qla4xxx/ql4_mbx.c |  2 +-
 drivers/scsi/qla4xxx/ql4_os.c  | 96 ++++++++++++++++++------------------------
 2 files changed, 42 insertions(+), 56 deletions(-)

-- 
2.2.2

[PATCH 1/4] SCSI-QLA4...: Deletion of unnecessary checks before three function calls

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 6 Feb 2015 20:38:23 +0100

The following functions perform also input parameter validation.
* iscsi_boot_destroy_kset
* kfree
* vfree

Thus the test around their calls is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/scsi/qla4xxx/ql4_mbx.c |  2 +-
 drivers/scsi/qla4xxx/ql4_os.c  | 31 ++++++++++---------------------
 2 files changed, 11 insertions(+), 22 deletions(-)

diff --git a/drivers/scsi/qla4xxx/ql4_mbx.c b/drivers/scsi/qla4xxx/ql4_mbx.c
index c291fdf..2343c0f 100644
--- a/drivers/scsi/qla4xxx/ql4_mbx.c
+++ b/drivers/scsi/qla4xxx/ql4_mbx.c
@@ -2410,7 +2410,7 @@ exit_free_acb:
 	dma_free_coherent(&ha->pdev->dev, sizeof(struct addr_ctrl_blk), acb,
 			  acb_dma);
 exit_config_acb:
-	if ((acb_config == ACB_CONFIG_SET) && ha->saved_acb) {
+	if (acb_config == ACB_CONFIG_SET) {
 		kfree(ha->saved_acb);
 		ha->saved_acb = NULL;
 	}
diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
index 6d25879..2a00fd3 100644
--- a/drivers/scsi/qla4xxx/ql4_os.c
+++ b/drivers/scsi/qla4xxx/ql4_os.c
@@ -4131,9 +4131,7 @@ static void qla4xxx_mem_free(struct scsi_qla_host *ha)
 		dma_free_coherent(&ha->pdev->dev, ha->queues_len, ha->queues,
 				  ha->queues_dma);
 
-	 if (ha->fw_dump)
-		vfree(ha->fw_dump);
-
+	vfree(ha->fw_dump);
 	ha->queues_len = 0;
 	ha->queues = NULL;
 	ha->queues_dma = 0;
@@ -4155,8 +4153,7 @@ static void qla4xxx_mem_free(struct scsi_qla_host *ha)
 	if (ha->chap_dma_pool)
 		dma_pool_destroy(ha->chap_dma_pool);
 
-	if (ha->chap_list)
-		vfree(ha->chap_list);
+	vfree(ha->chap_list);
 	ha->chap_list = NULL;
 
 	if (ha->fw_ddb_dma_pool)
@@ -4175,9 +4172,7 @@ static void qla4xxx_mem_free(struct scsi_qla_host *ha)
 		iounmap(ha->reg);
 	}
 
-	if (ha->reset_tmplt.buff)
-		vfree(ha->reset_tmplt.buff);
-
+	vfree(ha->reset_tmplt.buff);
 	pci_release_regions(ha->pdev);
 }
 
@@ -6370,10 +6365,8 @@ static int qla4xxx_is_session_exists(struct scsi_qla_host *ha,
 	}
 
 exit_check:
-	if (fw_tddb)
-		vfree(fw_tddb);
-	if (tmp_tddb)
-		vfree(tmp_tddb);
+	vfree(fw_tddb);
+	vfree(tmp_tddb);
 	return ret;
 }
 
@@ -6525,10 +6518,8 @@ static int qla4xxx_is_flash_ddb_exists(struct scsi_qla_host *ha,
 	}
 
 exit_check:
-	if (fw_tddb)
-		vfree(fw_tddb);
-	if (tmp_tddb)
-		vfree(tmp_tddb);
+	vfree(fw_tddb);
+	vfree(tmp_tddb);
 	return ret;
 }
 
@@ -7806,10 +7797,8 @@ static int qla4xxx_sysfs_ddb_logout(struct iscsi_bus_flash_session *fnode_sess,
 		ret = -ESRCH;
 
 exit_ddb_logout:
-	if (flash_tddb)
-		vfree(flash_tddb);
-	if (tmp_tddb)
-		vfree(tmp_tddb);
+	vfree(flash_tddb);
+	vfree(tmp_tddb);
 	if (fw_ddb_entry)
 		dma_pool_free(ha->fw_ddb_dma_pool, fw_ddb_entry, fw_ddb_dma);
 
@@ -9002,7 +8991,7 @@ static void qla4xxx_remove_adapter(struct pci_dev *pdev)
 	/* destroy iface from sysfs */
 	qla4xxx_destroy_ifaces(ha);
 
-	if ((!ql4xdisablesysfsboot) && ha->boot_kset)
+	if (!ql4xdisablesysfsboot)
 		iscsi_boot_destroy_kset(ha->boot_kset);
 
 	qla4xxx_destroy_fw_ddb_session(ha);
-- 
2.2.2

[PATCH 2/4] SCSI-QLA4...: Less function calls in qla4xxx_is_session_exists() after error detection

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 6 Feb 2015 21:14:40 +0100

The vfree() function was called in two cases by the qla4xxx_is_session_exists()
function during error handling even if the passed variables "fw_tddb" and
"tmp_tddb" contained still a null pointer.

* This implementation detail could be improved by adjustments for jump labels.

* Let us return immediately after the first failed function call according to
  the current Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/scsi/qla4xxx/ql4_os.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
index 2a00fd3..a7ca479 100644
--- a/drivers/scsi/qla4xxx/ql4_os.c
+++ b/drivers/scsi/qla4xxx/ql4_os.c
@@ -6327,17 +6327,15 @@ static int qla4xxx_is_session_exists(struct scsi_qla_host *ha,
 				     uint32_t *index)
 {
 	struct ddb_entry *ddb_entry;
-	struct ql4_tuple_ddb *fw_tddb = NULL;
-	struct ql4_tuple_ddb *tmp_tddb = NULL;
 	int idx;
 	int ret = QLA_ERROR;
+	struct ql4_tuple_ddb *tmp_tddb;
+	struct ql4_tuple_ddb *fw_tddb = vzalloc(sizeof(*fw_tddb));
 
-	fw_tddb = vzalloc(sizeof(*fw_tddb));
 	if (!fw_tddb) {
 		DEBUG2(ql4_printk(KERN_WARNING, ha,
 				  "Memory Allocation failed.\n"));
-		ret = QLA_SUCCESS;
-		goto exit_check;
+		return QLA_SUCCESS;
 	}
 
 	tmp_tddb = vzalloc(sizeof(*tmp_tddb));
@@ -6345,7 +6343,7 @@ static int qla4xxx_is_session_exists(struct scsi_qla_host *ha,
 		DEBUG2(ql4_printk(KERN_WARNING, ha,
 				  "Memory Allocation failed.\n"));
 		ret = QLA_SUCCESS;
-		goto exit_check;
+		goto free_fw;
 	}
 
 	qla4xxx_convert_param_ddb(fw_ddb_entry, fw_tddb, NULL);
@@ -6360,13 +6358,14 @@ static int qla4xxx_is_session_exists(struct scsi_qla_host *ha,
 			ret = QLA_SUCCESS; /* found */
 			if (index != NULL)
 				*index = idx;
-			goto exit_check;
+			goto free_tmp;
 		}
 	}
 
-exit_check:
-	vfree(fw_tddb);
+free_tmp:
 	vfree(tmp_tddb);
+free_fw:
+	vfree(fw_tddb);
 	return ret;
 }
 
-- 
2.2.2

[PATCH 3/4] SCSI-QLA4...: Less function calls in qla4xxx_is_flash_ddb_exists() after error detection

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 6 Feb 2015 21:55:55 +0100

The vfree() function was called in two cases by the
qla4xxx_is_flash_ddb_exists() function during error handling even if the passed
variables "fw_tddb" and "tmp_tddb" contained still a null pointer.

* This implementation detail could be improved by adjustments for jump labels.

* Let us return immediately after the first failed function call according to
  the current Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/scsi/qla4xxx/ql4_os.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
index a7ca479..e508bc9 100644
--- a/drivers/scsi/qla4xxx/ql4_os.c
+++ b/drivers/scsi/qla4xxx/ql4_os.c
@@ -6470,16 +6470,14 @@ static int qla4xxx_is_flash_ddb_exists(struct scsi_qla_host *ha,
 				       struct dev_db_entry *fw_ddb_entry)
 {
 	struct qla_ddb_index  *nt_ddb_idx, *nt_ddb_idx_tmp;
-	struct ql4_tuple_ddb *fw_tddb = NULL;
-	struct ql4_tuple_ddb *tmp_tddb = NULL;
 	int rval, ret = QLA_ERROR;
+	struct ql4_tuple_ddb *tmp_tddb;
+	struct ql4_tuple_ddb *fw_tddb = vzalloc(sizeof(*fw_tddb));
 
-	fw_tddb = vzalloc(sizeof(*fw_tddb));
 	if (!fw_tddb) {
 		DEBUG2(ql4_printk(KERN_WARNING, ha,
 				  "Memory Allocation failed.\n"));
-		ret = QLA_SUCCESS;
-		goto exit_check;
+		return QLA_SUCCESS;
 	}
 
 	tmp_tddb = vzalloc(sizeof(*tmp_tddb));
@@ -6487,7 +6485,7 @@ static int qla4xxx_is_flash_ddb_exists(struct scsi_qla_host *ha,
 		DEBUG2(ql4_printk(KERN_WARNING, ha,
 				  "Memory Allocation failed.\n"));
 		ret = QLA_SUCCESS;
-		goto exit_check;
+		goto vfree_fw;
 	}
 
 	qla4xxx_convert_param_ddb(fw_ddb_entry, fw_tddb, NULL);
@@ -6498,7 +6496,7 @@ static int qla4xxx_is_flash_ddb_exists(struct scsi_qla_host *ha,
 		ret = qla4xxx_compare_tuple_ddb(ha, fw_tddb, tmp_tddb, true);
 		/* found duplicate ddb */
 		if (ret == QLA_SUCCESS)
-			goto exit_check;
+			goto vfree_tmp;
 	}
 
 	list_for_each_entry_safe(nt_ddb_idx, nt_ddb_idx_tmp, list_nt, list) {
@@ -6512,13 +6510,14 @@ static int qla4xxx_is_flash_ddb_exists(struct scsi_qla_host *ha,
 			else
 				ret = QLA_SUCCESS;
 
-			goto exit_check;
+			goto vfree_tmp;
 		}
 	}
 
-exit_check:
-	vfree(fw_tddb);
+vfree_tmp:
 	vfree(tmp_tddb);
+vfree_fw:
+	vfree(fw_tddb);
 	return ret;
 }
 
-- 
2.2.2

[PATCH 4/4] SCSI-QLA4...: Less function calls in qla4xxx_sysfs_ddb_logout() after error detection

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 6 Feb 2015 22:44:23 +0100

The vfree() function was called in three cases by the qla4xxx_sysfs_ddb_logout()
function during error handling even if the passed variables "flash_tddb" and
"tmp_tddb" contained still a null pointer.

* This implementation detail could be improved by adjustments for jump labels.

* Let us return immediately after the first failed function call according to
  the current Linux coding style convention.

* Delete also an unnecessary check for the variable "fw_ddb_entry" there.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/scsi/qla4xxx/ql4_os.c | 35 +++++++++++++++++------------------
 1 file changed, 17 insertions(+), 18 deletions(-)

diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
index e508bc9..802390b 100644
--- a/drivers/scsi/qla4xxx/ql4_os.c
+++ b/drivers/scsi/qla4xxx/ql4_os.c
@@ -7699,23 +7699,22 @@ static int qla4xxx_sysfs_ddb_logout(struct iscsi_bus_flash_session *fnode_sess,
 {
 	struct Scsi_Host *shost = iscsi_flash_session_to_shost(fnode_sess);
 	struct scsi_qla_host *ha = to_qla_host(shost);
-	struct ql4_tuple_ddb *flash_tddb = NULL;
-	struct ql4_tuple_ddb *tmp_tddb = NULL;
-	struct dev_db_entry *fw_ddb_entry = NULL;
-	struct ddb_entry *ddb_entry = NULL;
 	dma_addr_t fw_ddb_dma;
 	uint32_t next_idx = 0;
 	uint32_t state = 0, conn_err = 0;
 	uint16_t conn_id = 0;
 	int idx, index;
 	int status, ret = 0;
+	struct ql4_tuple_ddb *flash_tddb;
+	struct ql4_tuple_ddb *tmp_tddb;
+	struct ddb_entry *ddb_entry;
+	struct dev_db_entry *fw_ddb_entry = dma_pool_alloc(ha->fw_ddb_dma_pool,
+								GFP_KERNEL,
+								&fw_ddb_dma);
 
-	fw_ddb_entry = dma_pool_alloc(ha->fw_ddb_dma_pool, GFP_KERNEL,
-				      &fw_ddb_dma);
-	if (fw_ddb_entry == NULL) {
+	if (!fw_ddb_entry) {
 		ql4_printk(KERN_ERR, ha, "%s:Out of memory\n", __func__);
-		ret = -ENOMEM;
-		goto exit_ddb_logout;
+		return -ENOMEM;
 	}
 
 	flash_tddb = vzalloc(sizeof(*flash_tddb));
@@ -7723,7 +7722,7 @@ static int qla4xxx_sysfs_ddb_logout(struct iscsi_bus_flash_session *fnode_sess,
 		ql4_printk(KERN_WARNING, ha,
 			   "%s:Memory Allocation failed.\n", __func__);
 		ret = -ENOMEM;
-		goto exit_ddb_logout;
+		goto free_pool;
 	}
 
 	tmp_tddb = vzalloc(sizeof(*tmp_tddb));
@@ -7731,7 +7730,7 @@ static int qla4xxx_sysfs_ddb_logout(struct iscsi_bus_flash_session *fnode_sess,
 		ql4_printk(KERN_WARNING, ha,
 			   "%s:Memory Allocation failed.\n", __func__);
 		ret = -ENOMEM;
-		goto exit_ddb_logout;
+		goto free_flash;
 	}
 
 	if (!fnode_sess->targetname) {
@@ -7739,7 +7738,7 @@ static int qla4xxx_sysfs_ddb_logout(struct iscsi_bus_flash_session *fnode_sess,
 			   "%s:Cannot logout from SendTarget entry\n",
 			   __func__);
 		ret = -EPERM;
-		goto exit_ddb_logout;
+		goto free_tmp;
 	}
 
 	if (fnode_sess->is_boot_target) {
@@ -7747,7 +7746,7 @@ static int qla4xxx_sysfs_ddb_logout(struct iscsi_bus_flash_session *fnode_sess,
 			   "%s: Logout from boot target entry is not permitted.\n",
 			   __func__);
 		ret = -EPERM;
-		goto exit_ddb_logout;
+		goto free_tmp;
 	}
 
 	strlcpy(flash_tddb->iscsi_name, fnode_sess->targetname,
@@ -7794,12 +7793,12 @@ static int qla4xxx_sysfs_ddb_logout(struct iscsi_bus_flash_session *fnode_sess,
 	if (idx == MAX_DDB_ENTRIES)
 		ret = -ESRCH;
 
-exit_ddb_logout:
-	vfree(flash_tddb);
+free_tmp:
 	vfree(tmp_tddb);
-	if (fw_ddb_entry)
-		dma_pool_free(ha->fw_ddb_dma_pool, fw_ddb_entry, fw_ddb_dma);
-
+free_flash:
+	vfree(flash_tddb);
+free_pool:
+	dma_pool_free(ha->fw_ddb_dma_pool, fw_ddb_entry, fw_ddb_dma);
 	return ret;
 }
 
-- 
2.2.2

Re: [PATCH 2/4] SCSI-QLA4...: Less function calls in qla4xxx_is_session_exists() after error detection

[Dan Carpenter]

On Fri, Feb 06, 2015 at 11:15:13PM +0100, SF Markus Elfring wrote:
> diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
> index 2a00fd3..a7ca479 100644
> --- a/drivers/scsi/qla4xxx/ql4_os.c
> +++ b/drivers/scsi/qla4xxx/ql4_os.c
> @@ -6327,17 +6327,15 @@ static int qla4xxx_is_session_exists(struct scsi_qla_host *ha,
>  				     uint32_t *index)
>  {
>  	struct ddb_entry *ddb_entry;
> -	struct ql4_tuple_ddb *fw_tddb = NULL;
> -	struct ql4_tuple_ddb *tmp_tddb = NULL;
>  	int idx;
>  	int ret = QLA_ERROR;
> +	struct ql4_tuple_ddb *tmp_tddb;
> +	struct ql4_tuple_ddb *fw_tddb = vzalloc(sizeof(*fw_tddb));
>  

Don't do allocations in the initializers.  Same for patches 3 and 4 as
well.

regards,
dan carpenter

Re: [PATCH 2/4] SCSI-QLA4...: Less function calls in qla4xxx_is_session_exists() after error detection

[Julia Lawall]

On Sat, 7 Feb 2015, Dan Carpenter wrote:

> On Fri, Feb 06, 2015 at 11:15:13PM +0100, SF Markus Elfring wrote:
> > diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
> > index 2a00fd3..a7ca479 100644
> > --- a/drivers/scsi/qla4xxx/ql4_os.c
> > +++ b/drivers/scsi/qla4xxx/ql4_os.c
> > @@ -6327,17 +6327,15 @@ static int qla4xxx_is_session_exists(struct scsi_qla_host *ha,
> >  				     uint32_t *index)
> >  {
> >  	struct ddb_entry *ddb_entry;
> > -	struct ql4_tuple_ddb *fw_tddb = NULL;
> > -	struct ql4_tuple_ddb *tmp_tddb = NULL;
> >  	int idx;
> >  	int ret = QLA_ERROR;
> > +	struct ql4_tuple_ddb *tmp_tddb;
> > +	struct ql4_tuple_ddb *fw_tddb = vzalloc(sizeof(*fw_tddb));
> >  
> 
> Don't do allocations in the initializers.  Same for patches 3 and 4 as
> well.

Why not?  I can think of some reasons, but I am wondering what is the 
precise one.

thanks,
julia

Re: [PATCH 2/4] SCSI-QLA4...: Less function calls in qla4xxx_is_session_exists() after error detection

[Dan Carpenter]

On Sat, Feb 07, 2015 at 11:11:03AM +0100, Julia Lawall wrote:
> On Sat, 7 Feb 2015, Dan Carpenter wrote:
> 
> > On Fri, Feb 06, 2015 at 11:15:13PM +0100, SF Markus Elfring wrote:
> > > diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
> > > index 2a00fd3..a7ca479 100644
> > > --- a/drivers/scsi/qla4xxx/ql4_os.c
> > > +++ b/drivers/scsi/qla4xxx/ql4_os.c
> > > @@ -6327,17 +6327,15 @@ static int qla4xxx_is_session_exists(struct scsi_qla_host *ha,
> > >  				     uint32_t *index)
> > >  {
> > >  	struct ddb_entry *ddb_entry;
> > > -	struct ql4_tuple_ddb *fw_tddb = NULL;
> > > -	struct ql4_tuple_ddb *tmp_tddb = NULL;
> > >  	int idx;
> > >  	int ret = QLA_ERROR;
> > > +	struct ql4_tuple_ddb *tmp_tddb;
> > > +	struct ql4_tuple_ddb *fw_tddb = vzalloc(sizeof(*fw_tddb));
> > >  
> > 
> > Don't do allocations in the initializers.  Same for patches 3 and 4 as
> > well.
> 
> Why not?  I can think of some reasons, but I am wondering what is the 
> precise one.

1)  People gloss over initializers without reading them.  You shouldn't
    put complicated code in initializers.  Sarah Sharp is more strict on
    this than I am.  http://www.spinics.net/lists/linux-usb/msg44389.html

2)  It means you have to put a line of separation between the allocation
    and the NULL check, but they should be next to each other.

regards,
dan carpenter

Re: [PATCH 2/4] SCSI-QLA4...: Less function calls in qla4xxx_is_session_exists() after error detection

[SF Markus Elfring]

> 1)  People gloss over initializers without reading them.

I would find such a habit strange.


>     You shouldn't put complicated code in initializers.

Does the affected memory allocation fall into such a category?


> 2)  It means you have to put a line of separation between the allocation
>     and the NULL check, but they should be next to each other.

Would you like to extend the Linux coding style documentation?

Regards,
Markus

Re: [PATCH 0/2] SCSI-lpfc: Deletion of an unnecessary check

[James Smart]

Markus,

Suggestions noted. If not merged, I'll pull them into the next patch set.

-- james


Acked-By: James Smart  <james.smart@emulex.com>


On 2/5/2015 5:25 PM, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Thu, 5 Feb 2015 23:17:52 +0100
>
> Another update suggestion was taken into account after a patch was applied
> from static source code analysis.
>
> Markus Elfring (2):
>    Delete an unnecessary check before the function call "kfree"
>    One function call less in lpfc_bsg_hba_set_event() after error detection
>
>   drivers/scsi/lpfc/lpfc_bsg.c | 7 +++----
>   1 file changed, 3 insertions(+), 4 deletions(-)
>

Re: [PATCH 1/3] SCSI-debug: Deletion of unnecessary checks before the function call "vfree"

[Douglas Gilbert]

On 15-02-06 12:23 PM, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Fri, 6 Feb 2015 16:56:57 +0100
>
> The vfree() function performs also input parameter validation.
> Thus the test around the call is not needed.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

Acked-by: Douglas Gilbert <dgilbert@interlog.com>

> ---
>   drivers/scsi/scsi_debug.c | 12 +++---------
>   1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index 1132321..9e4add7 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -4969,12 +4969,9 @@ bus_unreg:
>   dev_unreg:
>   	root_device_unregister(pseudo_primary);
>   free_vm:
> -	if (map_storep)
> -		vfree(map_storep);
> -	if (dif_storep)
> -		vfree(dif_storep);
> +	vfree(map_storep);
> +	vfree(dif_storep);
>   	vfree(fake_storep);
> -
>   	return ret;
>   }
>
> @@ -4989,10 +4986,7 @@ static void __exit scsi_debug_exit(void)
>   	driver_unregister(&sdebug_driverfs_driver);
>   	bus_unregister(&pseudo_lld_bus);
>   	root_device_unregister(pseudo_primary);
> -
> -	if (dif_storep)
> -		vfree(dif_storep);
> -
> +	vfree(dif_storep);
>   	vfree(fake_storep);
>   }
>
>

Re: [PATCH 2/3] SCSI-debug: Less function calls in scsi_debug_init() after error detection

[Douglas Gilbert]

On 15-02-06 12:25 PM, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Fri, 6 Feb 2015 17:48:34 +0100
>
> The vfree() function was called in two cases by the scsi_debug_init() function
> during error handling even if the passed variables "dif_storep" and
> "map_storep" contained null pointers eventually.
>
> This implementation detail could be improved by the introduction of two
> jump labels.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

Acked-by: Douglas Gilbert <dgilbert@interlog.com>

> ---
>   drivers/scsi/scsi_debug.c | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index 9e4add7..756b7be 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -4887,7 +4887,7 @@ static int __init scsi_debug_init(void)
>   		if (dif_storep == NULL) {
>   			pr_err("%s: out of mem. (DIX)\n", __func__);
>   			ret = -ENOMEM;
> -			goto free_vm;
> +			goto free_fake;
>   		}
>
>   		memset(dif_storep, 0xff, dif_size);
> @@ -4920,7 +4920,7 @@ static int __init scsi_debug_init(void)
>   		if (map_storep == NULL) {
>   			pr_err("%s: out of mem. (MAP)\n", __func__);
>   			ret = -ENOMEM;
> -			goto free_vm;
> +			goto free_dif;
>   		}
>
>   		bitmap_zero(map_storep, map_size);
> @@ -4934,7 +4934,7 @@ static int __init scsi_debug_init(void)
>   	if (IS_ERR(pseudo_primary)) {
>   		pr_warn("%s: root_device_register() error\n", __func__);
>   		ret = PTR_ERR(pseudo_primary);
> -		goto free_vm;
> +		goto free_map;
>   	}
>   	ret = bus_register(&pseudo_lld_bus);
>   	if (ret < 0) {
> @@ -4968,9 +4968,11 @@ bus_unreg:
>   	bus_unregister(&pseudo_lld_bus);
>   dev_unreg:
>   	root_device_unregister(pseudo_primary);
> -free_vm:
> +free_map:
>   	vfree(map_storep);
> +free_dif:
>   	vfree(dif_storep);
> +free_fake:
>   	vfree(fake_storep);
>   	return ret;
>   }
>

Re: [PATCH 3/3] SCSI-debug: Fix exception handling for an alignment/granularity mismatch in scsi_debug_init()

[Douglas Gilbert]

On 15-02-06 12:26 PM, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Fri, 6 Feb 2015 17:57:32 +0100
>
> The scsi_debug_init() function returned directly if the data items "alignment"
> and "granularity" did not fit together.
>
> Let us improve the affected exception handling by replacing the return
> statement by a goto statement so that previously allocated resources will also
> be appropriately released.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

Acked-by: Douglas Gilbert <dgilbert@interlog.com>

> ---
>   drivers/scsi/scsi_debug.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index 756b7be..2c78663 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -4909,7 +4909,8 @@ static int __init scsi_debug_init(void)
>   		    scsi_debug_unmap_alignment) {
>   			pr_err("%s: ERR: unmap_granularity <= unmap_alignment\n",
>   			       __func__);
> -			return -EINVAL;
> +			ret = -EINVAL;
> +			goto free_dif;
>   		}
>
>   		map_size = lba_to_map_index(sdebug_store_sectors - 1) + 1;
>

[PATCH] SCSI-OSD: Delete an unnecessary check before the function call "put_disk"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 24 Jun 2015 16:06:21 +0200

The put_disk() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/scsi/osd/osd_uld.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/scsi/osd/osd_uld.c b/drivers/scsi/osd/osd_uld.c
index 243eab3..e2075522 100644
--- a/drivers/scsi/osd/osd_uld.c
+++ b/drivers/scsi/osd/osd_uld.c
@@ -407,9 +407,7 @@ static void __remove(struct device *dev)
 
 	OSD_INFO("osd_remove %s\n",
 		 oud->disk ? oud->disk->disk_name : NULL);
-
-	if (oud->disk)
-		put_disk(oud->disk);
+	put_disk(oud->disk);
 	ida_remove(&osd_minor_ida, oud->minor);
 
 	kfree(oud);
-- 
2.4.4

Re: [PATCH] SCSI-OSD: Delete an unnecessary check before the function call "put_disk"

[Johannes Thumshirn]

On Wed, Jun 24, 2015 at 04:16:34PM +0200, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 24 Jun 2015 16:06:21 +0200
> 
> The put_disk() function tests whether its argument is NULL and then
> returns immediately. Thus the test around the call is not needed.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/scsi/osd/osd_uld.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/scsi/osd/osd_uld.c b/drivers/scsi/osd/osd_uld.c
> index 243eab3..e2075522 100644
> --- a/drivers/scsi/osd/osd_uld.c
> +++ b/drivers/scsi/osd/osd_uld.c
> @@ -407,9 +407,7 @@ static void __remove(struct device *dev)
>  
>  	OSD_INFO("osd_remove %s\n",
>  		 oud->disk ? oud->disk->disk_name : NULL);
> -
> -	if (oud->disk)
> -		put_disk(oud->disk);
> +	put_disk(oud->disk);
>  	ida_remove(&osd_minor_ida, oud->minor);
>  
>  	kfree(oud);
> -- 
> 2.4.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

-- 
Johannes Thumshirn                                       Storage
jthumshirn@suse.de                             +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] SCSI-wd33c93: Deletion of a check before the function call "wd33c93_setup"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 24 Jun 2015 17:15:17 +0200

The wd33c93_setup() function performs also input parameter validation.
Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/scsi/wd33c93.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/wd33c93.c b/drivers/scsi/wd33c93.c
index 9e09da4..e5ed956 100644
--- a/drivers/scsi/wd33c93.c
+++ b/drivers/scsi/wd33c93.c
@@ -1939,7 +1939,7 @@ wd33c93_init(struct Scsi_Host *instance, const wd33c93_regs regs,
 	int val;
 	char buf[32];
 
-	if (!done_setup && setup_strings)
+	if (!done_setup)
 		wd33c93_setup(setup_strings);
 
 	hostdata = (struct WD33C93_hostdata *) instance->hostdata;
-- 
2.4.4

[PATCH] SCSI-libfc: Delete an unnecessary check before the function call "fc_fcp_ddp_done"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 24 Jun 2015 17:50:32 +0200

The fc_fcp_ddp_done() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/scsi/libfc/fc_exch.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c
index 1b3a094..aad6f48 100644
--- a/drivers/scsi/libfc/fc_exch.c
+++ b/drivers/scsi/libfc/fc_exch.c
@@ -2120,8 +2120,7 @@ static struct fc_seq *fc_exch_seq_send(struct fc_lport *lport,
 	spin_unlock_bh(&ep->ex_lock);
 	return sp;
 err:
-	if (fsp)
-		fc_fcp_ddp_done(fsp);
+	fc_fcp_ddp_done(fsp);
 	rc = fc_exch_done_locked(ep);
 	spin_unlock_bh(&ep->ex_lock);
 	if (!rc)
-- 
2.4.4

[PATCH] SCSI: Delete unnecessary checks before the function call "put_device"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 24 Jun 2015 19:24:01 +0200

The put_device() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/scsi/hosts.c      | 4 +---
 drivers/scsi/scsi_sysfs.c | 4 +---
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index 8bb173e..84c8565 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -336,9 +336,7 @@ static void scsi_host_dev_release(struct device *dev)
 	}
 
 	kfree(shost->shost_data);
-
-	if (parent)
-		put_device(parent);
+	put_device(parent);
 	kfree(shost);
 }
 
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 1ac38e7..9ed5391 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -425,9 +425,7 @@ static void scsi_device_dev_release_usercontext(struct work_struct *work)
 	kfree(sdev->vpd_pg80);
 	kfree(sdev->inquiry);
 	kfree(sdev);
-
-	if (parent)
-		put_device(parent);
+	put_device(parent);
 }
 
 static void scsi_device_dev_release(struct device *dev)
-- 
2.4.4

[PATCH] SCSI-fnic: Delete an unnecessary check before the function call "vfree"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 24 Jun 2015 20:40:35 +0200

The vfree() function performs also input parameter validation.
Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/scsi/fnic/fnic_debugfs.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/scsi/fnic/fnic_debugfs.c b/drivers/scsi/fnic/fnic_debugfs.c
index d6498fa..855d382 100644
--- a/drivers/scsi/fnic/fnic_debugfs.c
+++ b/drivers/scsi/fnic/fnic_debugfs.c
@@ -102,9 +102,7 @@ void fnic_debugfs_terminate(void)
 
 	debugfs_remove(fnic_trace_debugfs_root);
 	fnic_trace_debugfs_root = NULL;
-
-	if (fc_trc_flag)
-		vfree(fc_trc_flag);
+	vfree(fc_trc_flag);
 }
 
 /*
-- 
2.4.4

Re: [PATCH] SCSI-wd33c93: Deletion of a check before the function call "wd33c93_setup"

[Dan Carpenter]

On Wed, Jun 24, 2015 at 05:33:50PM +0200, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 24 Jun 2015 17:15:17 +0200
> 
> The wd33c93_setup() function performs also input parameter validation.
> Thus the test around the call is not needed.
> 

Guys, you should be aware that Markus auto generates his patches and he
doesn't even do a cursory review.

"I find it acceptable that some of my update suggestions do not fit
to your quality expectations at the moment." -- Markus Elfring
(https://lkml.org/lkml/2015/1/22/446)

regards,
dan carpenter

Re: [PATCH] SCSI-libfc: Delete an unnecessary check before the function call "fc_fcp_ddp_done"

[Vasu Dev]

On Wed, 2015-06-24 at 17:54 +0200, SF Markus Elfring wrote:
> From: Markus Elfring <elfring-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
> Date: Wed, 24 Jun 2015 17:50:32 +0200
> 
> The fc_fcp_ddp_done() function tests whether its argument is NULL and then
> returns immediately. Thus the test around the call is not needed.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
> ---
>  drivers/scsi/libfc/fc_exch.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c
> index 1b3a094..aad6f48 100644
> --- a/drivers/scsi/libfc/fc_exch.c
> +++ b/drivers/scsi/libfc/fc_exch.c
> @@ -2120,8 +2120,7 @@ static struct fc_seq *fc_exch_seq_send(struct fc_lport *lport,
>  	spin_unlock_bh(&ep->ex_lock);
>  	return sp;
>  err:
> -	if (fsp)
> -		fc_fcp_ddp_done(fsp);
> +	fc_fcp_ddp_done(fsp);
>  	rc = fc_exch_done_locked(ep);
>  	spin_unlock_bh(&ep->ex_lock);
>  	if (!rc)

Looks good.

Acked-by: Vasu Dev <vasu.dev-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

Re: [PATCH] SCSI-OSD: Delete an unnecessary check before the function call "put_disk"

[Boaz Harrosh]

On 06/24/2015 05:16 PM, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 24 Jun 2015 16:06:21 +0200
> 
> The put_disk() function tests whether its argument is NULL and then
> returns immediately. Thus the test around the call is not needed.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

ACK-by: Boaz Harrosh <ooo@electrozaur.com>

> ---
>  drivers/scsi/osd/osd_uld.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/scsi/osd/osd_uld.c b/drivers/scsi/osd/osd_uld.c
> index 243eab3..e2075522 100644
> --- a/drivers/scsi/osd/osd_uld.c
> +++ b/drivers/scsi/osd/osd_uld.c
> @@ -407,9 +407,7 @@ static void __remove(struct device *dev)
>  
>  	OSD_INFO("osd_remove %s\n",
>  		 oud->disk ? oud->disk->disk_name : NULL);
> -
> -	if (oud->disk)
> -		put_disk(oud->disk);
> +	put_disk(oud->disk);
>  	ida_remove(&osd_minor_ida, oud->minor);
>  
>  	kfree(oud);
> 

Re: [PATCH] SCSI-bfa: Deletion of an unnecessary check before the function call "vfree"

[SF Markus Elfring]

> Patch looks good. 
> Thanks!
> Acked-by: Anil Gurumurthy <anil.gurumurthy@qlogic.com>
> 
> -----Original Message-----
> From: SF Markus Elfring [mailto:elfring@users.sourceforge.net] 
> Sent: 21 November 2014 04:17
> To: Anil Gurumurthy; James E. J. Bottomley; Sudarsana Kalluru; linux-scsi
> Cc: linux-kernel; kernel-janitors@vger.kernel.org; Julia Lawall
> Subject: [PATCH 1/1] SCSI-bfa: Deletion of an unnecessary check before the function call "vfree"
> 
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Thu, 20 Nov 2014 23:43:17 +0100
> 
> The vfree() function performs also input parameter validation. Thus the test around the call is not needed.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/scsi/bfa/bfad_debugfs.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/bfa/bfad_debugfs.c b/drivers/scsi/bfa/bfad_debugfs.c index 8e83d04..a72170f 100644
> --- a/drivers/scsi/bfa/bfad_debugfs.c
> +++ b/drivers/scsi/bfa/bfad_debugfs.c
> @@ -399,8 +399,7 @@ bfad_debugfs_release_fwtrc(struct inode *inode, struct file *file)
>  	if (!fw_debug)
>  		return 0;
>  
> -	if (fw_debug->debug_buffer)
> -		vfree(fw_debug->debug_buffer);
> +	vfree(fw_debug->debug_buffer);
>  
>  	file->private_data = NULL;
>  	kfree(fw_debug);
> --
> 2.1.3
> 

Would you like to integrate this update suggestion
into another source code repository?

Regards,
Markus

Re: [PATCH] SCSI-aic94xx: Deletion of an unnecessary check before the function call "kfree"

[SF Markus Elfring]

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Thu, 20 Nov 2014 23:18:56 +0100
> 
> The kfree() function tests whether its argument is NULL and then
> returns immediately. Thus the test around the call is not needed.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/scsi/aic94xx/aic94xx_init.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/aic94xx/aic94xx_init.c b/drivers/scsi/aic94xx/aic94xx_init.c
> index c56741f..0bfdcba 100644
> --- a/drivers/scsi/aic94xx/aic94xx_init.c
> +++ b/drivers/scsi/aic94xx/aic94xx_init.c
> @@ -597,8 +597,7 @@ static void asd_destroy_ha_caches(struct asd_ha_struct *asd_ha)
>  	if (asd_ha->hw_prof.scb_ext)
>  		asd_free_coherent(asd_ha, asd_ha->hw_prof.scb_ext);
>  
> -	if (asd_ha->hw_prof.ddb_bitmap)
> -		kfree(asd_ha->hw_prof.ddb_bitmap);
> +	kfree(asd_ha->hw_prof.ddb_bitmap);
>  	asd_ha->hw_prof.ddb_bitmap = NULL;
>  
>  	for (i = 0; i < ASD_MAX_PHYS; i++) {
> 

How are the chances to integrate this update suggestion
into another source code repository?

Regards,
Markus

Re: [PATCH] SCSI-eata_pio: Deletion of an unnecessary check before the function call "pci_dev_put"

[SF Markus Elfring]

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Thu, 20 Nov 2014 20:37:30 +0100
> 
> The pci_dev_put() function tests whether its argument is NULL and then
> returns immediately. Thus the test around the call is not needed.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/scsi/eata_pio.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/eata_pio.c b/drivers/scsi/eata_pio.c
> index 8319d2b..707f64d 100644
> --- a/drivers/scsi/eata_pio.c
> +++ b/drivers/scsi/eata_pio.c
> @@ -122,8 +122,7 @@ static int eata_pio_release(struct Scsi_Host *sh)
>  			release_region(sh->io_port, sh->n_io_port);
>  	}
>  	/* At this point the PCI reference can go */
> -	if (hd->pdev)
> -		pci_dev_put(hd->pdev);
> +	pci_dev_put(hd->pdev);
>  	return 1;
>  }
>  
> 

Would you like to integrate this update suggestion
into another source code repository?

Regards,
Markus

Re: [PATCH] SCSI-aic7...: Delete unnecessary checks before the function call "kfree"

[SF Markus Elfring]

Am 05.02.2015 um 22:27 schrieb SF Markus Elfring:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Thu, 5 Feb 2015 22:23:48 +0100
> 
> The kfree() function tests whether its argument is NULL and then
> returns immediately. Thus the test around the call is not needed.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/scsi/aic7xxx/aic7xxx_core.c | 15 +++++----------
>  1 file changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/scsi/aic7xxx/aic7xxx_core.c b/drivers/scsi/aic7xxx/aic7xxx_core.c
> index 10172a3..98a5ed1 100644
> --- a/drivers/scsi/aic7xxx/aic7xxx_core.c
> +++ b/drivers/scsi/aic7xxx/aic7xxx_core.c
> @@ -2200,8 +2200,7 @@ ahc_free_tstate(struct ahc_softc *ahc, u_int scsi_id, char channel, int force)
>  	if (channel == 'B')
>  		scsi_id += 8;
>  	tstate = ahc->enabled_targets[scsi_id];
> -	if (tstate != NULL)
> -		kfree(tstate);
> +	kfree(tstate);
>  	ahc->enabled_targets[scsi_id] = NULL;
>  }
>  #endif
> @@ -4482,8 +4481,7 @@ ahc_set_unit(struct ahc_softc *ahc, int unit)
>  void
>  ahc_set_name(struct ahc_softc *ahc, char *name)
>  {
> -	if (ahc->name != NULL)
> -		kfree(ahc->name);
> +	kfree(ahc->name);
>  	ahc->name = name;
>  }
>  
> @@ -4550,10 +4548,8 @@ ahc_free(struct ahc_softc *ahc)
>  		kfree(ahc->black_hole);
>  	}
>  #endif
> -	if (ahc->name != NULL)
> -		kfree(ahc->name);
> -	if (ahc->seep_config != NULL)
> -		kfree(ahc->seep_config);
> +	kfree(ahc->name);
> +	kfree(ahc->seep_config);
>  #ifndef __FreeBSD__
>  	kfree(ahc);
>  #endif
> @@ -4958,8 +4954,7 @@ ahc_fini_scbdata(struct ahc_softc *ahc)
>  	case 0:
>  		break;
>  	}
> -	if (scb_data->scbarray != NULL)
> -		kfree(scb_data->scbarray);
> +	kfree(scb_data->scbarray);
>  }
>  
>  static void
> 

Would you like to integrate this update suggestion
into another source code repository?

Regards,
Markus

Re: [PATCH] SCSI-libcxgbi: Deletion of an unnecessary check before the function call "dst_release"

[SF Markus Elfring]

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Fri, 21 Nov 2014 09:15:10 +0100
> 
> The dst_release() function tests whether its argument is NULL and then
> returns immediately. Thus the test around the call is not needed.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/scsi/cxgbi/libcxgbi.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/cxgbi/libcxgbi.c b/drivers/scsi/cxgbi/libcxgbi.c
> index addd1dd..95d2654 100644
> --- a/drivers/scsi/cxgbi/libcxgbi.c
> +++ b/drivers/scsi/cxgbi/libcxgbi.c
> @@ -798,8 +798,7 @@ void cxgbi_sock_closed(struct cxgbi_sock *csk)
>  		return;
>  	if (csk->saddr.sin_port)
>  		sock_put_port(csk);
> -	if (csk->dst)
> -		dst_release(csk->dst);
> +	dst_release(csk->dst);
>  	csk->cdev->csk_release_offload_resources(csk);
>  	cxgbi_sock_set_state(csk, CTP_CLOSED);
>  	cxgbi_inform_iscsi_conn_closing(csk);
> 

Would you like to integrate this update suggestion
into another source code repository?

Regards,
Markus

[PATCH] SCSI-libfc: Delete an unnecessary check before the function call "kmem_cache_destroy"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 16 Nov 2015 09:39:12 +0100

The kmem_cache_destroy() function tests whether its argument is NULL
and then returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/scsi/libfc/fc_fcp.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/scsi/libfc/fc_fcp.c b/drivers/scsi/libfc/fc_fcp.c
index 5121272..d377514 100644
--- a/drivers/scsi/libfc/fc_fcp.c
+++ b/drivers/scsi/libfc/fc_fcp.c
@@ -2216,8 +2216,7 @@ int fc_setup_fcp(void)
 
 void fc_destroy_fcp(void)
 {
-	if (scsi_pkt_cachep)
-		kmem_cache_destroy(scsi_pkt_cachep);
+	kmem_cache_destroy(scsi_pkt_cachep);
 }
 
 /**
-- 
2.6.2

[PATCH] SCSI-aic94xx: Delete unnecessary checks before the function call "kmem_cache_destroy"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 17 Nov 2015 08:14:52 +0100

The kmem_cache_destroy() function tests whether its argument is NULL
and then returns immediately. Thus the test around the calls is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/scsi/aic94xx/aic94xx_init.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/aic94xx/aic94xx_init.c b/drivers/scsi/aic94xx/aic94xx_init.c
index 662b232..ab93049 100644
--- a/drivers/scsi/aic94xx/aic94xx_init.c
+++ b/drivers/scsi/aic94xx/aic94xx_init.c
@@ -660,12 +660,9 @@ Err:
 
 static void asd_destroy_global_caches(void)
 {
-	if (asd_dma_token_cache)
-		kmem_cache_destroy(asd_dma_token_cache);
+	kmem_cache_destroy(asd_dma_token_cache);
 	asd_dma_token_cache = NULL;
-
-	if (asd_ascb_cache)
-		kmem_cache_destroy(asd_ascb_cache);
+	kmem_cache_destroy(asd_ascb_cache);
 	asd_ascb_cache = NULL;
 }
 
-- 
2.6.2

[PATCH] scsi_lib: Delete unnecessary checks before two function calls

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 17 Nov 2015 09:00:31 +0100

The functions kmem_cache_destroy() and mempool_destroy() test whether
their argument is NULL and then return immediately.
Thus the tests around their calls are not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/scsi/scsi_lib.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index dd8ad2a..13973a3 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2298,10 +2298,8 @@ int __init scsi_init_queue(void)
 cleanup_sdb:
 	for (i = 0; i < SG_MEMPOOL_NR; i++) {
 		struct scsi_host_sg_pool *sgp = scsi_sg_pools + i;
-		if (sgp->pool)
-			mempool_destroy(sgp->pool);
-		if (sgp->slab)
-			kmem_cache_destroy(sgp->slab);
+		mempool_destroy(sgp->pool);
+		kmem_cache_destroy(sgp->slab);
 	}
 	kmem_cache_destroy(scsi_sdb_cache);
 
-- 
2.6.2

[PATCH] SCSI-lpfc: Delete unnecessary checks before the function call "mempool_destroy"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 17 Nov 2015 09:34:27 +0100

The mempool_destroy() function tests whether its argument is NULL
and then returns immediately. Thus the test around the calls is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/scsi/lpfc/lpfc_mem.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_mem.c b/drivers/scsi/lpfc/lpfc_mem.c
index 3fa6533..4fb3581 100644
--- a/drivers/scsi/lpfc/lpfc_mem.c
+++ b/drivers/scsi/lpfc/lpfc_mem.c
@@ -231,15 +231,13 @@ lpfc_mem_free(struct lpfc_hba *phba)
 	if (phba->lpfc_hbq_pool)
 		pci_pool_destroy(phba->lpfc_hbq_pool);
 	phba->lpfc_hbq_pool = NULL;
-
-	if (phba->rrq_pool)
-		mempool_destroy(phba->rrq_pool);
+	mempool_destroy(phba->rrq_pool);
 	phba->rrq_pool = NULL;
 
 	/* Free NLP memory pool */
 	mempool_destroy(phba->nlp_mem_pool);
 	phba->nlp_mem_pool = NULL;
-	if (phba->sli_rev == LPFC_SLI_REV4 && phba->active_rrq_pool) {
+	if (phba->sli_rev == LPFC_SLI_REV4) {
 		mempool_destroy(phba->active_rrq_pool);
 		phba->active_rrq_pool = NULL;
 	}
-- 
2.6.2

Re: [PATCH] SCSI-libfc: Delete an unnecessary check before the function call "kmem_cache_destroy"

[Vasu Dev]

On Mon, 2015-11-16 at 09:45 +0100, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Mon, 16 Nov 2015 09:39:12 +0100
> 
> The kmem_cache_destroy() function tests whether its argument is NULL
> and then returns immediately. Thus the test around the call is not needed.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/scsi/libfc/fc_fcp.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/libfc/fc_fcp.c b/drivers/scsi/libfc/fc_fcp.c
> index 5121272..d377514 100644
> --- a/drivers/scsi/libfc/fc_fcp.c
> +++ b/drivers/scsi/libfc/fc_fcp.c
> @@ -2216,8 +2216,7 @@ int fc_setup_fcp(void)
>  
>  void fc_destroy_fcp(void)
>  {
> -	if (scsi_pkt_cachep)
> -		kmem_cache_destroy(scsi_pkt_cachep);
> +	kmem_cache_destroy(scsi_pkt_cachep);
>  }
>  
>  /**
Looks good.

Acked-by: Vasu Dev <vasu.dev@intel.com>

Re: [PATCH] SCSI-lpfc: Delete unnecessary checks before the function call "mempool_destroy"

[James Smart]

fyi - this patch was just pushed in our 11.0.0.10 patch set - patch 15/17

-- james s


On 11/17/2015 12:44 AM, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 17 Nov 2015 09:34:27 +0100
>
> The mempool_destroy() function tests whether its argument is NULL
> and then returns immediately. Thus the test around the calls is not needed.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>   drivers/scsi/lpfc/lpfc_mem.c | 6 ++----
>   1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/scsi/lpfc/lpfc_mem.c b/drivers/scsi/lpfc/lpfc_mem.c
> index 3fa6533..4fb3581 100644
> --- a/drivers/scsi/lpfc/lpfc_mem.c
> +++ b/drivers/scsi/lpfc/lpfc_mem.c
> @@ -231,15 +231,13 @@ lpfc_mem_free(struct lpfc_hba *phba)
>   	if (phba->lpfc_hbq_pool)
>   		pci_pool_destroy(phba->lpfc_hbq_pool);
>   	phba->lpfc_hbq_pool = NULL;
> -
> -	if (phba->rrq_pool)
> -		mempool_destroy(phba->rrq_pool);
> +	mempool_destroy(phba->rrq_pool);
>   	phba->rrq_pool = NULL;
>   
>   	/* Free NLP memory pool */
>   	mempool_destroy(phba->nlp_mem_pool);
>   	phba->nlp_mem_pool = NULL;
> -	if (phba->sli_rev == LPFC_SLI_REV4 && phba->active_rrq_pool) {
> +	if (phba->sli_rev == LPFC_SLI_REV4) {
>   		mempool_destroy(phba->active_rrq_pool);
>   		phba->active_rrq_pool = NULL;
>   	}

[PATCH 0/3] xen-scsiback: Fine-tuning for scsiback_device_action()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 16 Jul 2016 22:06:54 +0200

Further update suggestions were taken into account
after a patch was applied from static source code analysis.

Markus Elfring (3):
  Delete an unnecessary check before the function call "kfree"
  One function call less in scsiback_device_action() after error detection
  Pass a failure indication as a constant

 drivers/xen/xen-scsiback.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

-- 
2.9.1

[PATCH 1/3] xen-scsiback: Delete an unnecessary check before the function call "kfree"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 16 Jul 2016 21:21:05 +0200

The kfree() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/xen/xen-scsiback.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c
index d6950e0..4a48c06 100644
--- a/drivers/xen/xen-scsiback.c
+++ b/drivers/xen/xen-scsiback.c
@@ -627,8 +627,7 @@ static void scsiback_device_action(struct vscsibk_pend *pending_req,
 	transport_generic_free_cmd(&pending_req->se_cmd, 1);
 	return;
 err:
-	if (tmr)
-		kfree(tmr);
+	kfree(tmr);
 	scsiback_do_resp_with_sense(NULL, err, 0, pending_req);
 }
 
-- 
2.9.1

[PATCH 2/3] xen-scsiback: One function call less in scsiback_device_action() after error detection

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 16 Jul 2016 21:42:42 +0200

The kfree() function was called in one case by the
scsiback_device_action() function during error handling
even if the passed variable "tmr" contained a null pointer.

Adjust jump targets according to the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/xen/xen-scsiback.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c
index 4a48c06..7612bc9 100644
--- a/drivers/xen/xen-scsiback.c
+++ b/drivers/xen/xen-scsiback.c
@@ -606,7 +606,7 @@ static void scsiback_device_action(struct vscsibk_pend *pending_req,
 	tmr = kzalloc(sizeof(struct scsiback_tmr), GFP_KERNEL);
 	if (!tmr) {
 		target_put_sess_cmd(se_cmd);
-		goto err;
+		goto do_resp;
 	}
 
 	init_waitqueue_head(&tmr->tmr_wait);
@@ -616,7 +616,7 @@ static void scsiback_device_action(struct vscsibk_pend *pending_req,
 			       unpacked_lun, tmr, act, GFP_KERNEL,
 			       tag, TARGET_SCF_ACK_KREF);
 	if (rc)
-		goto err;
+		goto free_tmr;
 
 	wait_event(tmr->tmr_wait, atomic_read(&tmr->tmr_complete));
 
@@ -626,8 +626,9 @@ static void scsiback_device_action(struct vscsibk_pend *pending_req,
 	scsiback_do_resp_with_sense(NULL, err, 0, pending_req);
 	transport_generic_free_cmd(&pending_req->se_cmd, 1);
 	return;
-err:
+free_tmr:
 	kfree(tmr);
+do_resp:
 	scsiback_do_resp_with_sense(NULL, err, 0, pending_req);
 }
 
-- 
2.9.1

[PATCH 3/3] xen-scsiback: Pass a failure indication as a constant

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 16 Jul 2016 21:55:01 +0200

Pass the constant "FAILED" in a function call directly instead of
using an intialisation for a local variable.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/xen/xen-scsiback.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c
index 7612bc9..ec5546c 100644
--- a/drivers/xen/xen-scsiback.c
+++ b/drivers/xen/xen-scsiback.c
@@ -601,7 +601,7 @@ static void scsiback_device_action(struct vscsibk_pend *pending_req,
 	struct se_cmd *se_cmd = &pending_req->se_cmd;
 	struct scsiback_tmr *tmr;
 	u64 unpacked_lun = pending_req->v2p->lun;
-	int rc, err = FAILED;
+	int rc, err;
 
 	tmr = kzalloc(sizeof(struct scsiback_tmr), GFP_KERNEL);
 	if (!tmr) {
@@ -629,7 +629,7 @@ static void scsiback_device_action(struct vscsibk_pend *pending_req,
 free_tmr:
 	kfree(tmr);
 do_resp:
-	scsiback_do_resp_with_sense(NULL, err, 0, pending_req);
+	scsiback_do_resp_with_sense(NULL, FAILED, 0, pending_req);
 }
 
 /*
-- 
2.9.1

Re: [PATCH 1/3] xen-scsiback: Delete an unnecessary check before the function call "kfree"

[Juergen Gross]

On 16/07/16 22:22, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sat, 16 Jul 2016 21:21:05 +0200
> 
> The kfree() function tests whether its argument is NULL and then
> returns immediately. Thus the test around the call is not needed.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen

Re: [PATCH 3/3] xen-scsiback: Pass a failure indication as a constant

[Juergen Gross]

On 16/07/16 22:24, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sat, 16 Jul 2016 21:55:01 +0200
> 
> Pass the constant "FAILED" in a function call directly instead of
> using an intialisation for a local variable.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen

Re: [PATCH 2/3] xen-scsiback: One function call less in scsiback_device_action() after error detection

[Juergen Gross]

On 16/07/16 22:23, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sat, 16 Jul 2016 21:42:42 +0200
> 
> The kfree() function was called in one case by the
> scsiback_device_action() function during error handling
> even if the passed variable "tmr" contained a null pointer.
> 
> Adjust jump targets according to the Linux coding style convention.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/xen/xen-scsiback.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c
> index 4a48c06..7612bc9 100644
> --- a/drivers/xen/xen-scsiback.c
> +++ b/drivers/xen/xen-scsiback.c
> @@ -606,7 +606,7 @@ static void scsiback_device_action(struct vscsibk_pend *pending_req,
>  	tmr = kzalloc(sizeof(struct scsiback_tmr), GFP_KERNEL);
>  	if (!tmr) {
>  		target_put_sess_cmd(se_cmd);
> -		goto err;
> +		goto do_resp;
>  	}

Hmm, I'm not convinced this is an improvement.

I'd rather rename the new error label to "put_cmd" and get rid of the
braces in above if statement:

-	if (!tmr) {
-		target_put_sess_cmd(se_cmd);
-		goto err;
-	}
+	if (!tmr)
+		goto put_cmd;

and then in the error path:

-err:
+put_cmd:
+	target_put_sess_cmd(se_cmd);
+free_tmr:
	kfree(tmr);


Juergen

>  
>  	init_waitqueue_head(&tmr->tmr_wait);
> @@ -616,7 +616,7 @@ static void scsiback_device_action(struct vscsibk_pend *pending_req,
>  			       unpacked_lun, tmr, act, GFP_KERNEL,
>  			       tag, TARGET_SCF_ACK_KREF);
>  	if (rc)
> -		goto err;
> +		goto free_tmr;
>  
>  	wait_event(tmr->tmr_wait, atomic_read(&tmr->tmr_complete));
>  
> @@ -626,8 +626,9 @@ static void scsiback_device_action(struct vscsibk_pend *pending_req,
>  	scsiback_do_resp_with_sense(NULL, err, 0, pending_req);
>  	transport_generic_free_cmd(&pending_req->se_cmd, 1);
>  	return;
> -err:
> +free_tmr:
>  	kfree(tmr);
> +do_resp:
>  	scsiback_do_resp_with_sense(NULL, err, 0, pending_req);
>  }
>  
> 

Re: [PATCH 2/3] xen-scsiback: One function call less in scsiback_device_action() after error detection

[SF Markus Elfring]

>> @@ -606,7 +606,7 @@ static void scsiback_device_action(struct vscsibk_pend *pending_req,
>>  	tmr = kzalloc(sizeof(struct scsiback_tmr), GFP_KERNEL);
>>  	if (!tmr) {
>>  		target_put_sess_cmd(se_cmd);
>> -		goto err;
>> +		goto do_resp;
>>  	}
> 
> Hmm, I'm not convinced this is an improvement.
> 
> I'd rather rename the new error label to "put_cmd" and get rid of the
> braces in above if statement:
> 
> -	if (!tmr) {
> -		target_put_sess_cmd(se_cmd);
> -		goto err;
> -	}
> +	if (!tmr)
> +		goto put_cmd;
> 
> and then in the error path:
> 
> -err:
> +put_cmd:
> +	target_put_sess_cmd(se_cmd);

I am unsure on the relevance of this function on such a source position.
Would it make sense to move it further down at the end?


> +free_tmr:
> 	kfree(tmr);

How do you think about to skip this function call after a memory
allocation failure?

Regards,
Markus

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 2/3] xen-scsiback: One function call less in scsiback_device_action() after error detection

[Juergen Gross]

On 19/07/16 16:56, SF Markus Elfring wrote:
>>> @@ -606,7 +606,7 @@ static void scsiback_device_action(struct vscsibk_pend *pending_req,
>>>  	tmr = kzalloc(sizeof(struct scsiback_tmr), GFP_KERNEL);
>>>  	if (!tmr) {
>>>  		target_put_sess_cmd(se_cmd);
>>> -		goto err;
>>> +		goto do_resp;
>>>  	}
>>
>> Hmm, I'm not convinced this is an improvement.
>>
>> I'd rather rename the new error label to "put_cmd" and get rid of the
>> braces in above if statement:
>>
>> -	if (!tmr) {
>> -		target_put_sess_cmd(se_cmd);
>> -		goto err;
>> -	}
>> +	if (!tmr)
>> +		goto put_cmd;
>>
>> and then in the error path:
>>
>> -err:
>> +put_cmd:
>> +	target_put_sess_cmd(se_cmd);
> 
> I am unsure on the relevance of this function on such a source position.
> Would it make sense to move it further down at the end?

You only want to call it in the first error case (allocation failure).

>> +free_tmr:
>> 	kfree(tmr);
> 
> How do you think about to skip this function call after a memory
> allocation failure?

I think this just doesn't matter. If it were a hot path, yes. But trying
to do micro-optimizations in an error path is just not worth the effort.

I like a linear error path containing all the needed cleanups best.


Juergen

Re: [PATCH 2/3] xen-scsiback: One function call less in scsiback_device_action() after error detection

[SF Markus Elfring]

>>>> @@ -606,7 +606,7 @@ static void scsiback_device_action(struct vscsibk_pend *pending_req,
>>>>  	tmr = kzalloc(sizeof(struct scsiback_tmr), GFP_KERNEL);
>>>>  	if (!tmr) {
>>>>  		target_put_sess_cmd(se_cmd);
>>>> -		goto err;
>>>> +		goto do_resp;
>>>>  	}
>>>
>>> Hmm, I'm not convinced this is an improvement.
>>>
>>> I'd rather rename the new error label to "put_cmd" and get rid of the
>>> braces in above if statement:
>>>
>>> -	if (!tmr) {
>>> -		target_put_sess_cmd(se_cmd);
>>> -		goto err;
>>> -	}
>>> +	if (!tmr)
>>> +		goto put_cmd;
>>>
>>> and then in the error path:
>>>
>>> -err:
>>> +put_cmd:
>>> +	target_put_sess_cmd(se_cmd);
>>
>> I am unsure on the relevance of this function on such a source position.
>> Would it make sense to move it further down at the end?
> 
> You only want to call it in the first error case (allocation failure).

Thanks for your clarification.

I find that my update suggestion (from Saturday) is still appropriate
in this case.
https://lkml.org/lkml/2016/7/16/172


>>> +free_tmr:
>>> 	kfree(tmr);
>>
>> How do you think about to skip this function call after a memory
>> allocation failure?
> 
> I think this just doesn't matter. If it were a hot path, yes. But trying
> to do micro-optimizations in an error path is just not worth the effort.

Would you like to reduce also the amount of function calls in such special
run-time situations?


> I like a linear error path containing all the needed cleanups best.

I would prefer to keep the discussed single function call within
the basic block of the if statement.

Have we got different opinions about the shown implementation details?

Regards,
Markus

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 2/3] xen-scsiback: One function call less in scsiback_device_action() after error detection

[Juergen Gross]

On 20/07/16 07:10, SF Markus Elfring wrote:
>>>>> @@ -606,7 +606,7 @@ static void scsiback_device_action(struct vscsibk_pend *pending_req,
>>>>>  	tmr = kzalloc(sizeof(struct scsiback_tmr), GFP_KERNEL);
>>>>>  	if (!tmr) {
>>>>>  		target_put_sess_cmd(se_cmd);
>>>>> -		goto err;
>>>>> +		goto do_resp;
>>>>>  	}
>>>>
>>>> Hmm, I'm not convinced this is an improvement.
>>>>
>>>> I'd rather rename the new error label to "put_cmd" and get rid of the
>>>> braces in above if statement:
>>>>
>>>> -	if (!tmr) {
>>>> -		target_put_sess_cmd(se_cmd);
>>>> -		goto err;
>>>> -	}
>>>> +	if (!tmr)
>>>> +		goto put_cmd;
>>>>
>>>> and then in the error path:
>>>>
>>>> -err:
>>>> +put_cmd:
>>>> +	target_put_sess_cmd(se_cmd);
>>>
>>> I am unsure on the relevance of this function on such a source position.
>>> Would it make sense to move it further down at the end?
>>
>> You only want to call it in the first error case (allocation failure).
> 
> Thanks for your clarification.
> 
> I find that my update suggestion (from Saturday) is still appropriate
> in this case.
> https://lkml.org/lkml/2016/7/16/172

And I still think it isn't an improvement: Nack

>>>> +free_tmr:
>>>> 	kfree(tmr);
>>>
>>> How do you think about to skip this function call after a memory
>>> allocation failure?
>>
>> I think this just doesn't matter. If it were a hot path, yes. But trying
>> to do micro-optimizations in an error path is just not worth the effort.
> 
> Would you like to reduce also the amount of function calls in such special
> run-time situations?

I just don't care for the extra 2 or 3 nsecs. Readability is more
important here.

>> I like a linear error path containing all the needed cleanups best.
> 
> I would prefer to keep the discussed single function call within
> the basic block of the if statement.
> 
> Have we got different opinions about the shown implementation details?

Yes.


Juergen

[PATCH v2 0/3] xen-scsiback: Fine-tuning for scsiback_device_action()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 20 Jul 2016 13:20:04 +0200

Further update suggestions were taken into account
after a patch was applied from static source code analysis.

Markus Elfring (3):
  Delete an unnecessary check before the function call "kfree"
  Rename jump labels in scsiback_device_action()
  Pass a failure indication as a constant

 drivers/xen/xen-scsiback.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

-- 
2.9.2


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v2 1/3] xen-scsiback: Delete an unnecessary check before the function call "kfree"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 19 Jul 2016 15:42:19 +0200

The kfree() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
v2: Rebased on source files from "Linux next-20160719"

 drivers/xen/xen-scsiback.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c
index d6950e0..4a48c06 100644
--- a/drivers/xen/xen-scsiback.c
+++ b/drivers/xen/xen-scsiback.c
@@ -627,8 +627,7 @@ static void scsiback_device_action(struct vscsibk_pend *pending_req,
 	transport_generic_free_cmd(&pending_req->se_cmd, 1);
 	return;
 err:
-	if (tmr)
-		kfree(tmr);
+	kfree(tmr);
 	scsiback_do_resp_with_sense(NULL, err, 0, pending_req);
 }
 
-- 
2.9.2


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v2 2/3] xen-scsiback: Rename jump labels in scsiback_device_action()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 20 Jul 2016 13:03:16 +0200

* Adjust jump targets according to the Linux coding style convention.

* A bit of refactoring for the control flow

Suggested-by: Jürgen Groß <jgross@suse.com>
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
v2: Rebased on source files from "Linux next-20160719"
    Changes from a bit of code review

 drivers/xen/xen-scsiback.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c
index 4a48c06..eb274df 100644
--- a/drivers/xen/xen-scsiback.c
+++ b/drivers/xen/xen-scsiback.c
@@ -604,10 +604,8 @@ static void scsiback_device_action(struct vscsibk_pend *pending_req,
 	int rc, err = FAILED;
 
 	tmr = kzalloc(sizeof(struct scsiback_tmr), GFP_KERNEL);
-	if (!tmr) {
-		target_put_sess_cmd(se_cmd);
-		goto err;
-	}
+	if (!tmr)
+		goto put_cmd;
 
 	init_waitqueue_head(&tmr->tmr_wait);
 
@@ -616,7 +614,7 @@ static void scsiback_device_action(struct vscsibk_pend *pending_req,
 			       unpacked_lun, tmr, act, GFP_KERNEL,
 			       tag, TARGET_SCF_ACK_KREF);
 	if (rc)
-		goto err;
+		goto free_tmr;
 
 	wait_event(tmr->tmr_wait, atomic_read(&tmr->tmr_complete));
 
@@ -626,7 +624,9 @@ static void scsiback_device_action(struct vscsibk_pend *pending_req,
 	scsiback_do_resp_with_sense(NULL, err, 0, pending_req);
 	transport_generic_free_cmd(&pending_req->se_cmd, 1);
 	return;
-err:
+put_cmd:
+	target_put_sess_cmd(se_cmd);
+free_tmr:
 	kfree(tmr);
 	scsiback_do_resp_with_sense(NULL, err, 0, pending_req);
 }
-- 
2.9.2


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v2 3/3] xen-scsiback: Pass a failure indication as a constant

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 20 Jul 2016 13:12:33 +0200

Pass the constant "FAILED" in a function call directly instead of
using an intialisation for a local variable.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
v2: Rebased on source files from "Linux next-20160719"

 drivers/xen/xen-scsiback.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c
index eb274df..fa08ec6 100644
--- a/drivers/xen/xen-scsiback.c
+++ b/drivers/xen/xen-scsiback.c
@@ -601,7 +601,7 @@ static void scsiback_device_action(struct vscsibk_pend *pending_req,
 	struct se_cmd *se_cmd = &pending_req->se_cmd;
 	struct scsiback_tmr *tmr;
 	u64 unpacked_lun = pending_req->v2p->lun;
-	int rc, err = FAILED;
+	int rc, err;
 
 	tmr = kzalloc(sizeof(struct scsiback_tmr), GFP_KERNEL);
 	if (!tmr)
@@ -628,7 +628,7 @@ put_cmd:
 	target_put_sess_cmd(se_cmd);
 free_tmr:
 	kfree(tmr);
-	scsiback_do_resp_with_sense(NULL, err, 0, pending_req);
+	scsiback_do_resp_with_sense(NULL, FAILED, 0, pending_req);
 }
 
 /*
-- 
2.9.2


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v2 1/3] xen-scsiback: Delete an unnecessary check before the function call "kfree"

[Juergen Gross]

On 20/07/16 13:30, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 19 Jul 2016 15:42:19 +0200
> 
> The kfree() function tests whether its argument is NULL and then
> returns immediately. Thus the test around the call is not needed.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

Even if already given, here it is again:

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen

Re: [PATCH v2 3/3] xen-scsiback: Pass a failure indication as a constant

[Juergen Gross]

On 20/07/16 13:36, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 20 Jul 2016 13:12:33 +0200
> 
> Pass the constant "FAILED" in a function call directly instead of
> using an intialisation for a local variable.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

Even if already given, here it is again:

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen

Re: [PATCH v2 2/3] xen-scsiback: Rename jump labels in scsiback_device_action()

[Juergen Gross]

On 20/07/16 13:34, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 20 Jul 2016 13:03:16 +0200
> 
> * Adjust jump targets according to the Linux coding style convention.
> 
> * A bit of refactoring for the control flow
> 
> Suggested-by: Jürgen Groß <jgross@suse.com>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] SCSI-aic94xx: Delete unnecessary checks before the function call "kmem_cache_destroy"

[SF Markus Elfring]

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 17 Nov 2015 08:14:52 +0100
> 
> The kmem_cache_destroy() function tests whether its argument is NULL
> and then returns immediately. Thus the test around the calls is not needed.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/scsi/aic94xx/aic94xx_init.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/scsi/aic94xx/aic94xx_init.c b/drivers/scsi/aic94xx/aic94xx_init.c
> index 662b232..ab93049 100644
> --- a/drivers/scsi/aic94xx/aic94xx_init.c
> +++ b/drivers/scsi/aic94xx/aic94xx_init.c
> @@ -660,12 +660,9 @@ Err:
>  
>  static void asd_destroy_global_caches(void)
>  {
> -	if (asd_dma_token_cache)
> -		kmem_cache_destroy(asd_dma_token_cache);
> +	kmem_cache_destroy(asd_dma_token_cache);
>  	asd_dma_token_cache = NULL;
> -
> -	if (asd_ascb_cache)
> -		kmem_cache_destroy(asd_ascb_cache);
> +	kmem_cache_destroy(asd_ascb_cache);
>  	asd_ascb_cache = NULL;
>  }
>  
> 

How do you think about to integrate this update suggestion
into another source code repository?

Regards,
Markus

[PATCH] scsi: Delete an unnecessary check before the function call "kfree"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 24 Jul 2016 14:20:21 +0200

The kfree() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/scsi/scsi.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 1f36aca..1794c0c 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -864,8 +864,7 @@ retry_pg83:
 		rcu_assign_pointer(sdev->vpd_pg83, vpd_buf);
 		mutex_unlock(&sdev->inquiry_mutex);
 		synchronize_rcu();
-		if (orig_vpd_buf)
-			kfree(orig_vpd_buf);
+		kfree(orig_vpd_buf);
 	}
 }
 
-- 
2.9.2

Re: [PATCH] scsi: Delete an unnecessary check before the function call "kfree"

[Laurence Oberman]

----- Original Message -----
> From: "SF Markus Elfring" <elfring@users.sourceforge.net>
> To: linux-scsi@vger.kernel.org, "Christoph Hellwig" <hch@lst.de>, "Hannes Reinecke" <hare@suse.de>, "James E. J.
> Bottomley" <jejb@linux.vnet.ibm.com>, "Martin K. Petersen" <martin.petersen@oracle.com>
> Cc: "LKML" <linux-kernel@vger.kernel.org>, kernel-janitors@vger.kernel.org, "Julia Lawall" <julia.lawall@lip6.fr>
> Sent: Sunday, July 24, 2016 8:30:35 AM
> Subject: [PATCH] scsi: Delete an unnecessary check before the function call "kfree"
> 
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 24 Jul 2016 14:20:21 +0200
> 
> The kfree() function tests whether its argument is NULL and then
> returns immediately. Thus the test around the call is not needed.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/scsi/scsi.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
> index 1f36aca..1794c0c 100644
> --- a/drivers/scsi/scsi.c
> +++ b/drivers/scsi/scsi.c
> @@ -864,8 +864,7 @@ retry_pg83:
>  		rcu_assign_pointer(sdev->vpd_pg83, vpd_buf);
>  		mutex_unlock(&sdev->inquiry_mutex);
>  		synchronize_rcu();
> -		if (orig_vpd_buf)
> -			kfree(orig_vpd_buf);
> +		kfree(orig_vpd_buf);
>  	}
>  }
>  
> --
> 2.9.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Looks fine, small comment is that the function call prior to check in the fucntion sets up variables etc. 
So is more expensive than a simple NULL check prior.

Reviewed-by: Laurence Oberman <loberman@redhat.com>

Re: [PATCH] SCSI-aic94xx: Delete unnecessary checks before the function call "kmem_cache_destroy"

[Jinpu Wang]

On Sun, Jul 24, 2016 at 1:51 PM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
>
> > From: Markus Elfring <elfring@users.sourceforge.net>
> > Date: Tue, 17 Nov 2015 08:14:52 +0100
> >
> > The kmem_cache_destroy() function tests whether its argument is NULL
> > and then returns immediately. Thus the test around the calls is not needed.
> >
> > This issue was detected by using the Coccinelle software.
> >
> > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> > ---
> >  drivers/scsi/aic94xx/aic94xx_init.c | 7 ++-----
> >  1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/scsi/aic94xx/aic94xx_init.c b/drivers/scsi/aic94xx/aic94xx_init.c
> > index 662b232..ab93049 100644
> > --- a/drivers/scsi/aic94xx/aic94xx_init.c
> > +++ b/drivers/scsi/aic94xx/aic94xx_init.c
> > @@ -660,12 +660,9 @@ Err:
> >
> >  static void asd_destroy_global_caches(void)
> >  {
> > -     if (asd_dma_token_cache)
> > -             kmem_cache_destroy(asd_dma_token_cache);
> > +     kmem_cache_destroy(asd_dma_token_cache);
> >       asd_dma_token_cache = NULL;
> > -
> > -     if (asd_ascb_cache)
> > -             kmem_cache_destroy(asd_ascb_cache);
> > +     kmem_cache_destroy(asd_ascb_cache);
> >       asd_ascb_cache = NULL;
> >  }
> >
> >
>
> How do you think about to integrate this update suggestion
> into another source code repository?
>
> Regards,
> Markus


Looks good to me!
Reviewed-by: Jack Wang <jinpu.wang@profitbricks.com>

PS: resend to fix mal-format rejection, sorry!
-- 

Mit freundlichen Grüßen,
Best Regards,

Jack Wang
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html