[PATCH 0/5] Fix several static checker warnings reported against the iSCSI kernel code

[PATCH 0/5] Fix several static checker warnings reported against the iSCSI kernel code

[Bart Van Assche]

Hello James and Martin,

These five patches are what I came up with after analyzing the output of 
"make M=drivers/scsi W=1 C=2" for the iSCSI initiator kernel code. 
Please consider these patches for inclusion in kernel v4.7. The actual 
patches are:

0001-libiscsi-Unexport-iscsi_eh_target_reset.patch
0002-libiscsi-Remove-set-but-not-used-variables.patch
0003-scsi_transport_iscsi-Remove-set-but-not-used-variabl.patch
0004-scsi_transport_iscsi-Unexport-iscsi_is_flashnode_con.patch
0005-scsi_transport_iscsi-Declare-local-symbols-static.patch

Thanks,

Bart.

[PATCH 1/5] libiscsi: Unexport iscsi_eh_target_reset()

[Bart Van Assche]

Running "git grep -nHw iscsi_eh_target_reset" shows that this
function is only called from inside the drivers/scsi/libiscsi.c
source file. Hence unexport this function.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Reviewed-by: Mike Christie <michaelc@cs.wisc.edu>
---
 drivers/scsi/libiscsi.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 6bffd91..120150a 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -2423,7 +2423,7 @@ static void iscsi_prep_tgt_reset_pdu(struct scsi_cmnd *sc, struct iscsi_tm *hdr)
  *
  * This will attempt to send a warm target reset.
  */
-int iscsi_eh_target_reset(struct scsi_cmnd *sc)
+static int iscsi_eh_target_reset(struct scsi_cmnd *sc)
 {
 	struct iscsi_cls_session *cls_session;
 	struct iscsi_session *session;
@@ -2495,7 +2495,6 @@ done:
 	mutex_unlock(&session->eh_mutex);
 	return rc;
 }
-EXPORT_SYMBOL_GPL(iscsi_eh_target_reset);
 
 /**
  * iscsi_eh_recover_target - reset target and possibly the session
-- 
2.7.3

[PATCH 2/5] libiscsi: Remove set-but-not-used variables

[Bart Van Assche]

Avoid that building with W=1 causes gcc to report warnings about
set-but-not-used variables.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Reviewed-by: Mike Christie <michaelc@cs.wisc.edu>
---
 drivers/scsi/libiscsi.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 120150a..c051694 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -2127,7 +2127,7 @@ int iscsi_eh_abort(struct scsi_cmnd *sc)
 	struct iscsi_conn *conn;
 	struct iscsi_task *task;
 	struct iscsi_tm *hdr;
-	int rc, age;
+	int age;
 
 	cls_session = starget_to_session(scsi_target(sc->device));
 	session = cls_session->dd_data;
@@ -2188,10 +2188,8 @@ int iscsi_eh_abort(struct scsi_cmnd *sc)
 	hdr = &conn->tmhdr;
 	iscsi_prep_abort_task_pdu(task, hdr);
 
-	if (iscsi_exec_task_mgmt_fn(conn, hdr, age, session->abort_timeout)) {
-		rc = FAILED;
+	if (iscsi_exec_task_mgmt_fn(conn, hdr, age, session->abort_timeout))
 		goto failed;
-	}
 
 	switch (conn->tmf_state) {
 	case TMF_SUCCESS:
-- 
2.7.3

[PATCH 3/5] scsi_transport_iscsi: Remove set-but-not-used variables

[Bart Van Assche]

Avoid that building with W=1 causes gcc to report warnings about
set-but-not-used variables.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Reviewed-by: Mike Christie <michaelc@cs.wisc.edu>
---
 drivers/scsi/scsi_transport_iscsi.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c
index 4414816..446781d 100644
--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -2067,13 +2067,10 @@ EXPORT_SYMBOL_GPL(iscsi_alloc_session);
 
 int iscsi_add_session(struct iscsi_cls_session *session, unsigned int target_id)
 {
-	struct Scsi_Host *shost = iscsi_session_to_shost(session);
-	struct iscsi_cls_host *ihost;
 	unsigned long flags;
 	int id = 0;
 	int err;
 
-	ihost = shost->shost_data;
 	session->sid = atomic_add_return(1, &iscsi_session_nr);
 
 	if (target_id == ISCSI_MAX_TARGET) {
-- 
2.7.3

[PATCH 4/5] scsi_transport_iscsi: Unexport iscsi_is_flashnode_conn_dev()

[Bart Van Assche]

The output of "git grep -nHw iscsi_is_flashnode_conn_dev" shows
that this function is only called from inside source file
drivers/scsi/scsi_transport_iscsi.c. Hence unexport this function.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Reviewed-by: Mike Christie <michaelc@cs.wisc.edu>
---
 drivers/scsi/scsi_transport_iscsi.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c
index 446781d..dff7413 100644
--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -1324,11 +1324,10 @@ EXPORT_SYMBOL_GPL(iscsi_create_flashnode_conn);
  *  1 on success
  *  0 on failure
  */
-int iscsi_is_flashnode_conn_dev(struct device *dev, void *data)
+static int iscsi_is_flashnode_conn_dev(struct device *dev, void *data)
 {
 	return dev->bus == &iscsi_flashnode_bus;
 }
-EXPORT_SYMBOL_GPL(iscsi_is_flashnode_conn_dev);
 
 static int iscsi_destroy_flashnode_conn(struct iscsi_bus_flash_conn *fnode_conn)
 {
-- 
2.7.3

[PATCH 5/5] scsi_transport_iscsi: Declare local symbols static

[Bart Van Assche]

Avoid that building with W=1 causes gcc to report warnings about
symbols that have not been declared.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Reviewed-by: Mike Christie <michaelc@cs.wisc.edu>
---
 drivers/scsi/scsi_transport_iscsi.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c
index dff7413..c169fa1 100644
--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -1009,7 +1009,7 @@ static void iscsi_flashnode_sess_release(struct device *dev)
 	kfree(fnode_sess);
 }
 
-struct device_type iscsi_flashnode_sess_dev_type = {
+static struct device_type iscsi_flashnode_sess_dev_type = {
 	.name = "iscsi_flashnode_sess_dev_type",
 	.groups = iscsi_flashnode_sess_attr_groups,
 	.release = iscsi_flashnode_sess_release,
@@ -1195,13 +1195,13 @@ static void iscsi_flashnode_conn_release(struct device *dev)
 	kfree(fnode_conn);
 }
 
-struct device_type iscsi_flashnode_conn_dev_type = {
+static struct device_type iscsi_flashnode_conn_dev_type = {
 	.name = "iscsi_flashnode_conn_dev_type",
 	.groups = iscsi_flashnode_conn_attr_groups,
 	.release = iscsi_flashnode_conn_release,
 };
 
-struct bus_type iscsi_flashnode_bus;
+static struct bus_type iscsi_flashnode_bus;
 
 int iscsi_flashnode_bus_match(struct device *dev,
 				     struct device_driver *drv)
@@ -1212,7 +1212,7 @@ int iscsi_flashnode_bus_match(struct device *dev,
 }
 EXPORT_SYMBOL_GPL(iscsi_flashnode_bus_match);
 
-struct bus_type iscsi_flashnode_bus = {
+static struct bus_type iscsi_flashnode_bus = {
 	.name = "iscsi_flashnode",
 	.match = &iscsi_flashnode_bus_match,
 };
-- 
2.7.3

Re: [PATCH 1/5] libiscsi: Unexport iscsi_eh_target_reset()

[Johannes Thumshirn]

On 2016-03-30 20:26, Bart Van Assche wrote:
> Running "git grep -nHw iscsi_eh_target_reset" shows that this
> function is only called from inside the drivers/scsi/libiscsi.c
> source file. Hence unexport this function.
> 
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> Reviewed-by: Mike Christie <michaelc@cs.wisc.edu>
> ---
>  drivers/scsi/libiscsi.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
> index 6bffd91..120150a 100644
> --- a/drivers/scsi/libiscsi.c
> +++ b/drivers/scsi/libiscsi.c
> @@ -2423,7 +2423,7 @@ static void iscsi_prep_tgt_reset_pdu(struct
> scsi_cmnd *sc, struct iscsi_tm *hdr)
>   *
>   * This will attempt to send a warm target reset.
>   */
> -int iscsi_eh_target_reset(struct scsi_cmnd *sc)
> +static int iscsi_eh_target_reset(struct scsi_cmnd *sc)
>  {
>  	struct iscsi_cls_session *cls_session;
>  	struct iscsi_session *session;
> @@ -2495,7 +2495,6 @@ done:
>  	mutex_unlock(&session->eh_mutex);
>  	return rc;
>  }
> -EXPORT_SYMBOL_GPL(iscsi_eh_target_reset);
> 
>  /**
>   * iscsi_eh_recover_target - reset target and possibly the session

Looks good to me,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

Re: [PATCH 2/5] libiscsi: Remove set-but-not-used variables

[Johannes Thumshirn]

On 2016-03-30 20:27, Bart Van Assche wrote:
> Avoid that building with W=1 causes gcc to report warnings about
> set-but-not-used variables.
> 
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> Reviewed-by: Mike Christie <michaelc@cs.wisc.edu>
> ---
>  drivers/scsi/libiscsi.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
> index 120150a..c051694 100644
> --- a/drivers/scsi/libiscsi.c
> +++ b/drivers/scsi/libiscsi.c
> @@ -2127,7 +2127,7 @@ int iscsi_eh_abort(struct scsi_cmnd *sc)
>  	struct iscsi_conn *conn;
>  	struct iscsi_task *task;
>  	struct iscsi_tm *hdr;
> -	int rc, age;
> +	int age;
> 
>  	cls_session = starget_to_session(scsi_target(sc->device));
>  	session = cls_session->dd_data;
> @@ -2188,10 +2188,8 @@ int iscsi_eh_abort(struct scsi_cmnd *sc)
>  	hdr = &conn->tmhdr;
>  	iscsi_prep_abort_task_pdu(task, hdr);
> 
> -	if (iscsi_exec_task_mgmt_fn(conn, hdr, age, session->abort_timeout)) 
> {
> -		rc = FAILED;
> +	if (iscsi_exec_task_mgmt_fn(conn, hdr, age, session->abort_timeout))
>  		goto failed;
> -	}
> 
>  	switch (conn->tmf_state) {
>  	case TMF_SUCCESS:

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

Re: [PATCH 3/5] scsi_transport_iscsi: Remove set-but-not-used variables

[Johannes Thumshirn]

On 2016-03-30 20:27, Bart Van Assche wrote:
> Avoid that building with W=1 causes gcc to report warnings about
> set-but-not-used variables.
> 
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> Reviewed-by: Mike Christie <michaelc@cs.wisc.edu>
> ---
>  drivers/scsi/scsi_transport_iscsi.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_transport_iscsi.c
> b/drivers/scsi/scsi_transport_iscsi.c
> index 4414816..446781d 100644
> --- a/drivers/scsi/scsi_transport_iscsi.c
> +++ b/drivers/scsi/scsi_transport_iscsi.c
> @@ -2067,13 +2067,10 @@ EXPORT_SYMBOL_GPL(iscsi_alloc_session);
> 
>  int iscsi_add_session(struct iscsi_cls_session *session, unsigned int
> target_id)
>  {
> -	struct Scsi_Host *shost = iscsi_session_to_shost(session);
> -	struct iscsi_cls_host *ihost;
>  	unsigned long flags;
>  	int id = 0;
>  	int err;
> 
> -	ihost = shost->shost_data;
>  	session->sid = atomic_add_return(1, &iscsi_session_nr);
> 
>  	if (target_id == ISCSI_MAX_TARGET) {

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

Re: [PATCH 4/5] scsi_transport_iscsi: Unexport iscsi_is_flashnode_conn_dev()

[Johannes Thumshirn]

On 2016-03-30 20:27, Bart Van Assche wrote:
> The output of "git grep -nHw iscsi_is_flashnode_conn_dev" shows
> that this function is only called from inside source file
> drivers/scsi/scsi_transport_iscsi.c. Hence unexport this function.
> 
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> Reviewed-by: Mike Christie <michaelc@cs.wisc.edu>
> ---
>  drivers/scsi/scsi_transport_iscsi.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_transport_iscsi.c
> b/drivers/scsi/scsi_transport_iscsi.c
> index 446781d..dff7413 100644
> --- a/drivers/scsi/scsi_transport_iscsi.c
> +++ b/drivers/scsi/scsi_transport_iscsi.c
> @@ -1324,11 +1324,10 @@ EXPORT_SYMBOL_GPL(iscsi_create_flashnode_conn);
>   *  1 on success
>   *  0 on failure
>   */
> -int iscsi_is_flashnode_conn_dev(struct device *dev, void *data)
> +static int iscsi_is_flashnode_conn_dev(struct device *dev, void *data)
>  {
>  	return dev->bus == &iscsi_flashnode_bus;
>  }
> -EXPORT_SYMBOL_GPL(iscsi_is_flashnode_conn_dev);
> 
>  static int iscsi_destroy_flashnode_conn(struct iscsi_bus_flash_conn
> *fnode_conn)
>  {

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

Re: [PATCH 5/5] scsi_transport_iscsi: Declare local symbols static

[Johannes Thumshirn]

On 2016-03-30 20:28, Bart Van Assche wrote:
> Avoid that building with W=1 causes gcc to report warnings about
> symbols that have not been declared.
> 
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> Reviewed-by: Mike Christie <michaelc@cs.wisc.edu>
> ---
>  drivers/scsi/scsi_transport_iscsi.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_transport_iscsi.c
> b/drivers/scsi/scsi_transport_iscsi.c
> index dff7413..c169fa1 100644
> --- a/drivers/scsi/scsi_transport_iscsi.c
> +++ b/drivers/scsi/scsi_transport_iscsi.c
> @@ -1009,7 +1009,7 @@ static void iscsi_flashnode_sess_release(struct
> device *dev)
>  	kfree(fnode_sess);
>  }
> 
> -struct device_type iscsi_flashnode_sess_dev_type = {
> +static struct device_type iscsi_flashnode_sess_dev_type = {
>  	.name = "iscsi_flashnode_sess_dev_type",
>  	.groups = iscsi_flashnode_sess_attr_groups,
>  	.release = iscsi_flashnode_sess_release,
> @@ -1195,13 +1195,13 @@ static void
> iscsi_flashnode_conn_release(struct device *dev)
>  	kfree(fnode_conn);
>  }
> 
> -struct device_type iscsi_flashnode_conn_dev_type = {
> +static struct device_type iscsi_flashnode_conn_dev_type = {
>  	.name = "iscsi_flashnode_conn_dev_type",
>  	.groups = iscsi_flashnode_conn_attr_groups,
>  	.release = iscsi_flashnode_conn_release,
>  };
> 
> -struct bus_type iscsi_flashnode_bus;
> +static struct bus_type iscsi_flashnode_bus;
> 
>  int iscsi_flashnode_bus_match(struct device *dev,
>  				     struct device_driver *drv)
> @@ -1212,7 +1212,7 @@ int iscsi_flashnode_bus_match(struct device *dev,
>  }
>  EXPORT_SYMBOL_GPL(iscsi_flashnode_bus_match);
> 
> -struct bus_type iscsi_flashnode_bus = {
> +static struct bus_type iscsi_flashnode_bus = {
>  	.name = "iscsi_flashnode",
>  	.match = &iscsi_flashnode_bus_match,
>  };

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

Re: [PATCH 0/5] Fix several static checker warnings reported against the iSCSI kernel code

[Martin K. Petersen]

>>>>> "Bart" == Bart Van Assche <bart.vanassche@sandisk.com> writes:

Bart> These five patches are what I came up with after analyzing the
Bart> output of "make M=drivers/scsi W=1 C=2" for the iSCSI initiator
Bart> kernel code. Please consider these patches for inclusion in kernel
Bart> v4.7. The actual patches are:

Bart> 0001-libiscsi-Unexport-iscsi_eh_target_reset.patch
Bart> 0002-libiscsi-Remove-set-but-not-used-variables.patch
Bart> 0003-scsi_transport_iscsi-Remove-set-but-not-used-variabl.patch
Bart> 0004-scsi_transport_iscsi-Unexport-iscsi_is_flashnode_con.patch
Bart> 0005-scsi_transport_iscsi-Declare-local-symbols-static.patch

Applied to 4.7/scsi-queue.

Thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering