Fix several static checker warnings reported against the iSCSI kernel code
cover Fix several static checker warnings reported against the iSCSI kernel code
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()
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.
--- drivers/scsi/libiscsi.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/scsi/libiscsi.c +1 -2
@@ -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
Signature · 1 line
2.7.3
Re: [PATCH 1/5] libiscsi: Unexport iscsi_eh_target_reset()
Quoted 25 lines · Bart Van Assche
Running "git grep -nHw
iscsi_eh_target_reset" shows that this function is only called from inside thedrivers/scsi/libiscsi.csource file. Hence unexport this function.Signed-off-by Bart Van Assche <[email protected]>Reviewed-by Mike Christie <[email protected]>--- drivers/scsi/libiscsi.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)drivers/scsi/libiscsi.c +1
-1@@ -2423,7 +2423,7 @@ static void iscsi_prep_tgt_reset_pdu(structscsi_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;drivers/scsi/libiscsi.c +0
-0struct iscsi_session *session;
drivers/scsi/libiscsi.c done: · line 2500
Quoted 7 lines
drivers/scsi/libiscsi.c +0
-1@@ -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,
PATCH 2/5 libiscsi: Remove set-but-not-used variables
In reply to Bart Van AsscheAvoid that building with W=1 causes gcc to report warnings about set-but-not-used variables.
--- drivers/scsi/libiscsi.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
drivers/scsi/libiscsi.c +2 -4
@@ -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 :
Signature · 1 line
2.7.3
Re: [PATCH 2/5] libiscsi: Remove set-but-not-used variables
Quoted 34 lines · 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 <[email protected]>Reviewed-by Mike Christie <[email protected]>--- drivers/scsi/libiscsi.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)drivers/scsi/libiscsi.c +2
-4@@ -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) {
Quoted 1 line
case
TMF_SUCCESS:
Looks good,
PATCH 3/5 scsi_transport_iscsi: Remove set-but-not-used variables
In reply to Bart Van AsscheAvoid that building with W=1 causes gcc to report warnings about set-but-not-used variables.
--- drivers/scsi/scsi_transport_iscsi.c | 3 --- 1 file changed, 3 deletions(-)
drivers/scsi/scsi_transport_iscsi.c +0 -3
@@ -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 ) {
Signature · 1 line
2.7.3
Re: [PATCH 3/5] scsi_transport_iscsi: Remove set-but-not-used variables
Quoted 28 lines · 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 <[email protected]>Reviewed-by Mike Christie <[email protected]>--- drivers/scsi/scsi_transport_iscsi.c | 3 --- 1 file changed, 3 deletions(-)drivers/scsi/scsi_transport_iscsi.c +0
-3@@ -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);
Quoted 1 line
if (
target_id==ISCSI_MAX_TARGET) {
Looks good,
PATCH 4/5 scsi_transport_iscsi: Unexport iscsi_is_flashnode_conn_dev()
In reply to Bart Van AsscheThe 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.
--- drivers/scsi/scsi_transport_iscsi.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/scsi/scsi_transport_iscsi.c +1 -2
@@ -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) {
Signature · 1 line
2.7.3
Re: [PATCH 4/5] scsi_transport_iscsi: Unexport iscsi_is_flashnode_conn_dev()
Quoted 28 lines · Bart Van Assche
The output of "git grep -nHw
iscsi_is_flashnode_conn_dev" shows that this function is only called from inside source filedrivers/scsi/scsi_transport_iscsi.c. Hence unexport this function.Signed-off-by Bart Van Assche <[email protected]>Reviewed-by Mike Christie <[email protected]>--- drivers/scsi/scsi_transport_iscsi.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)drivers/scsi/scsi_transport_iscsi.c +1
-2@@ -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)
Quoted 1 line
{
Looks good,
PATCH 5/5 scsi_transport_iscsi: Declare local symbols static
In reply to Bart Van AsscheAvoid that building with W=1 causes gcc to report warnings about symbols that have not been declared.
--- drivers/scsi/scsi_transport_iscsi.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
drivers/scsi/scsi_transport_iscsi.c +4 -4
@@ -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, };
Signature · 1 line
2.7.3
Re: [PATCH 5/5] scsi_transport_iscsi: Declare local symbols static
Quoted 42 lines · 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 <[email protected]>Reviewed-by Mike Christie <[email protected]>--- drivers/scsi/scsi_transport_iscsi.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)drivers/scsi/scsi_transport_iscsi.c +1
-1@@ -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,drivers/scsi/scsi_transport_iscsi.c +2
-2.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,drivers/scsi/scsi_transport_iscsi.c +0
-0struct device_driver *drv)
drivers/scsi/scsi_transport_iscsi.c int iscsi_flashnode_bus_match(struct device *dev, · line 1218
Quoted 8 lines
drivers/scsi/scsi_transport_iscsi.c +1
-1@@ -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,
Martin K. Petersen
AppliedQuoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Bart" == Bart Van Assche <[email protected]> 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!
Signature · 1 line
Martin K. Petersen Oracle Linux Engineering