Re: [2/4] scsi: hpsa: Less function calls in hpsa_big_passthru_ioctl() after error detection

[SF Markus Elfring]

>> @@ -6501,14 +6501,16 @@ static int hpsa_big_passthru_ioctl(struct ctlr_info
>> *h, void __user *argp)
>>  cleanup0:
>> cmd_free(h, c);
>>  cleanup1:
>> -   if (buff) {
>> +   {
>> int i;
>>
>> for (i = 0; i < sg_used; i++)
>> kfree(buff[i]);
>> kfree(buff);
>> }
> 
> Thanks for looking at the hpsa driver.
> 
> This HUNK ends up with an unnamed block.

Which identifier would you like to use there?


> I would prefer to not have it structured like this.

Would you like to show a source code layout alternative?

Regards,
Markus

[PATCH 4/4] scsi: hpsa: Move a variable assignment in hpsa_big_passthru_ioctl()

[SF Markus Elfring]

From: Markus Elfring 
Date: Sun, 4 Mar 2018 22:16:05 +0100

Move an assignment for the local variable "sg_used" so that its setting
will only be performed after corresponding memory allocations succeeded
by this function.

Signed-off-by: Markus Elfring 
---
 drivers/scsi/hpsa.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 86d371ab39e7..bb6df194ac31 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -6380,7 +6380,7 @@ static int hpsa_big_passthru_ioctl(struct ctlr_info *h, 
void __user *argp)
unsigned char **buff;
int *buff_size;
u64 temp64;
-   BYTE sg_used = 0;
+   BYTE sg_used;
int status;
u32 left;
u32 sz;
@@ -6420,6 +6420,7 @@ static int hpsa_big_passthru_ioctl(struct ctlr_info *h, 
void __user *argp)
}
left = ioc->buf_size;
data_ptr = ioc->buf;
+   sg_used = 0;
while (left) {
sz = (left > ioc->malloc_size) ? ioc->malloc_size : left;
buff_size[sg_used] = sz;
-- 
2.16.2

[PATCH 3/4] scsi: hpsa: Delete an unnecessary initialisation in hpsa_big_passthru_ioctl()

[SF Markus Elfring]

From: Markus Elfring 
Date: Sun, 4 Mar 2018 22:02:10 +0100

The variable "status" will be set to an appropriate value a bit later.
Thus omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring 
---
 drivers/scsi/hpsa.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 45177ead811f..86d371ab39e7 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -6381,7 +6381,7 @@ static int hpsa_big_passthru_ioctl(struct ctlr_info *h, 
void __user *argp)
int *buff_size;
u64 temp64;
BYTE sg_used = 0;
-   int status = 0;
+   int status;
u32 left;
u32 sz;
BYTE __user *data_ptr;
-- 
2.16.2

[PATCH 2/4] scsi: hpsa: Less function calls in hpsa_big_passthru_ioctl() after error detection

[SF Markus Elfring]

From: Markus Elfring 
Date: Sun, 4 Mar 2018 22:00:19 +0100

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

* Adjust jump targets.

* Delete two initialisations and a check (for the local variable "buff")
  which became unnecessary with this refactoring.

Signed-off-by: Markus Elfring 
---
 drivers/scsi/hpsa.c | 26 ++
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index b35248becef9..45177ead811f 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -6377,8 +6377,8 @@ static int hpsa_big_passthru_ioctl(struct ctlr_info *h, 
void __user *argp)
 {
BIG_IOCTL_Command_struct *ioc;
struct CommandList *c;
-   unsigned char **buff = NULL;
-   int *buff_size = NULL;
+   unsigned char **buff;
+   int *buff_size;
u64 temp64;
BYTE sg_used = 0;
int status = 0;
@@ -6397,26 +6397,26 @@ static int hpsa_big_passthru_ioctl(struct ctlr_info *h, 
void __user *argp)
if ((ioc->buf_size < 1) &&
(ioc->Request.Type.Direction != XFER_NONE)) {
status = -EINVAL;
-   goto cleanup1;
+   goto free_ioc;
}
/* Check kmalloc limits  using all SGs */
if (ioc->malloc_size > MAX_KMALLOC_SIZE) {
status = -EINVAL;
-   goto cleanup1;
+   goto free_ioc;
}
if (ioc->buf_size > ioc->malloc_size * SG_ENTRIES_IN_CMD) {
status = -EINVAL;
-   goto cleanup1;
-   }
-   buff = kzalloc(SG_ENTRIES_IN_CMD * sizeof(char *), GFP_KERNEL);
-   if (!buff) {
-   status = -ENOMEM;
-   goto cleanup1;
+   goto free_ioc;
}
buff_size = kmalloc(SG_ENTRIES_IN_CMD * sizeof(int), GFP_KERNEL);
if (!buff_size) {
status = -ENOMEM;
-   goto cleanup1;
+   goto free_ioc;
+   }
+   buff = kzalloc(SG_ENTRIES_IN_CMD * sizeof(char *), GFP_KERNEL);
+   if (!buff) {
+   status = -ENOMEM;
+   goto free_buff_size;
}
left = ioc->buf_size;
data_ptr = ioc->buf;
@@ -6501,14 +6501,16 @@ static int hpsa_big_passthru_ioctl(struct ctlr_info *h, 
void __user *argp)
 cleanup0:
cmd_free(h, c);
 cleanup1:
-   if (buff) {
+   {
int i;
 
for (i = 0; i < sg_used; i++)
kfree(buff[i]);
kfree(buff);
}
+free_buff_size:
kfree(buff_size);
+free_ioc:
kfree(ioc);
return status;
 }
-- 
2.16.2

[PATCH 1/4] scsi: hpsa: Use memdup_user() rather than duplicating its implementation

[SF Markus Elfring]

From: Markus Elfring 
Date: Sun, 4 Mar 2018 21:19:52 +0100

* Reuse existing functionality from memdup_user() instead of keeping
  duplicate source code.

  This issue was detected by using the Coccinelle software.

* Return directly after this function call failed at the beginning.

Signed-off-by: Markus Elfring 
---
 drivers/scsi/hpsa.c | 13 -
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 5293e6827ce5..b35248becef9 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -6390,15 +6390,10 @@ static int hpsa_big_passthru_ioctl(struct ctlr_info *h, 
void __user *argp)
return -EINVAL;
if (!capable(CAP_SYS_RAWIO))
return -EPERM;
-   ioc = kmalloc(sizeof(*ioc), GFP_KERNEL);
-   if (!ioc) {
-   status = -ENOMEM;
-   goto cleanup1;
-   }
-   if (copy_from_user(ioc, argp, sizeof(*ioc))) {
-   status = -EFAULT;
-   goto cleanup1;
-   }
+   ioc = memdup_user(argp, sizeof(*ioc));
+   if (IS_ERR(ioc))
+   return PTR_ERR(ioc);
+
if ((ioc->buf_size < 1) &&
(ioc->Request.Type.Direction != XFER_NONE)) {
status = -EINVAL;
-- 
2.16.2

[PATCH 0/4] SCSI-HPSA: Adjustments for hpsa_big_passthru_ioctl()

[SF Markus Elfring]

From: Markus Elfring 
Date: Mon, 5 Mar 2018 09:14:32 +0100

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (4):
  Use memdup_user() rather than duplicating its implementation
  Less function calls in hpsa_big_passthru_ioctl() after error detection
  Delete an unnecessary initialisation
  Move a variable assignment

 drivers/scsi/hpsa.c | 44 +---
 1 file changed, 21 insertions(+), 23 deletions(-)

-- 
2.16.2

Re: [0/8] target-iSCSI: Adjustments for several function implementations

[SF Markus Elfring]

>> Can a passed null pointer really work in this function?
>>
>> https://elixir.bootlin.com/linux/v4.16-rc2/source/include/crypto/hash.h#L684
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/crypto/hash.h?id=0f9da844d87796ac31b04e81ee95e155e9043132#n751
>>
>> static inline struct crypto_tfm *crypto_shash_tfm(struct crypto_shash *tfm)
>> {
>>  return >base;
>> }
> 
> Yes.  It's not a dereference,

Do any processors treat the zero address still special there?


> it's just doing pointer math to get the address.

Can eventually happen anything unexpected?


Can it be nicer to avoid such a software behaviour concern generally
just by adjusting a few jump labels (as I proposed it)?

Regards,
Markus

Re: [0/8] target-iSCSI: Adjustments for several function implementations

[SF Markus Elfring]

> Calling crypto_free_shash(NULL) is actually fine.

Really?


> It doesn't dereference the parameter, it just does pointer math on it in
> crypto_shash_tfm() and returns if it's NULL in crypto_destroy_tfm().

Can a passed null pointer really work in this function?

https://elixir.bootlin.com/linux/v4.16-rc2/source/include/crypto/hash.h#L684
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/crypto/hash.h?id=0f9da844d87796ac31b04e81ee95e155e9043132#n751

static inline struct crypto_tfm *crypto_shash_tfm(struct crypto_shash *tfm)
{
return >base;
}


Regards,
Markus

Re: [0/8] target-iSCSI: Adjustments for several function implementations

[SF Markus Elfring]

> You're 1/8 patch had an actual bug fix hidden amongst the style churn.

It showed the general possibility to adjust the source code structure
for the function “chap_server_compute_md5” also because of the usage
of the single jump label “out” before.


> I don't see any such fixes in the other patches.

This view is appropriate.

Further update steps show different transformation possibilities.


> My opinion from https://www.spinics.net/lists/target-devel/msg16342.html
> hasn't changed. FWIW, I'd prefer to see LIO adopt a policy similar to:
> https://btrfs.wiki.kernel.org/index.php/Developer%27s_FAQ#How_not_to_start

It seems that you express a few aspects for general change resistance.
Will the circumstances evolve for similar software improvements?

Regards,
Markus

Re: [PATCH 0/8] target-iSCSI: Adjustments for several function implementations

[SF Markus Elfring]

> Date: Tue, 12 Dec 2017 22:22:11 +0100
> 
> Some update suggestions were taken into account
> from static source code analysis.
> 
> Markus Elfring (8):
>   Less function calls in chap_server_compute_md5() after error detection
>   Move resetting of seven variables in chap_server_compute_md5()
>   Delete 36 error messages for a failed memory allocation
>   Delete an unnecessary variable initialisation in iscsit_allocate_ooo_cmdsn()
>   Delete an unnecessary variable initialisation in iscsi_copy_param_list()
>   Delete an unnecessary variable initialisation in 
> iscsi_create_default_params()
>   Delete an unnecessary variable initialisation in iscsi_set_default_param()
>   Improve 16 size determinations
> 
>  drivers/target/iscsi/iscsi_target.c   |   2 -
>  drivers/target/iscsi/iscsi_target_auth.c  | 110 
> +++---
>  drivers/target/iscsi/iscsi_target_datain_values.c |   6 +-
>  drivers/target/iscsi/iscsi_target_erl1.c  |  14 +--
>  drivers/target/iscsi/iscsi_target_erl2.c  |   8 +-
>  drivers/target/iscsi/iscsi_target_login.c |  29 ++
>  drivers/target/iscsi/iscsi_target_nego.c  |   4 +-
>  drivers/target/iscsi/iscsi_target_parameters.c|  58 +---
>  drivers/target/iscsi/iscsi_target_seq_pdu_list.c  |  34 +++
>  drivers/target/iscsi/iscsi_target_tpg.c   |  13 +--
>  drivers/target/iscsi/iscsi_target_util.c  |  11 +--
>  11 files changed, 118 insertions(+), 171 deletions(-)

One of these update suggestions resulted in the commit “target: avoid NULL
dereference in CHAP auth error path” which is considered for integration
into Linux stable versions now.
https://patchwork.kernel.org/patch/10110459/
https://lkml.kernel.org/r/<20171213172230.12767-1-dd...@suse.de>
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/target/iscsi/iscsi_target_auth.c?id=ce512d79d0466a604793addb6b769d12ee326822


Now I am curious if more remaining change possibilities can be picked up
from this patch series.

* Would you like to improve any more implementation details?

* Do you need additional explanations for further benefits?

Regards,
Markus

[PATCH 8/8] target/iscsi: Improve 16 size determinations

[SF Markus Elfring]

From: Markus Elfring 
Date: Tue, 12 Dec 2017 22:06:00 +0100

Replace the specification of data types by pointer dereferences
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/target/iscsi/iscsi_target_erl2.c |  2 +-
 drivers/target/iscsi/iscsi_target_login.c|  6 +++---
 drivers/target/iscsi/iscsi_target_parameters.c   | 10 +-
 drivers/target/iscsi/iscsi_target_seq_pdu_list.c | 11 ++-
 drivers/target/iscsi/iscsi_target_tpg.c  |  4 ++--
 5 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target_erl2.c 
b/drivers/target/iscsi/iscsi_target_erl2.c
index 87c27e8d4f49..80b8297f4b05 100644
--- a/drivers/target/iscsi/iscsi_target_erl2.c
+++ b/drivers/target/iscsi/iscsi_target_erl2.c
@@ -324,7 +324,7 @@ int iscsit_prepare_cmds_for_reallegiance(struct iscsi_conn 
*conn)
 * (struct iscsi_cmd->cr) so we need to allocate this before preparing 
the
 * connection's command list for connection recovery.
 */
-   cr = kzalloc(sizeof(struct iscsi_conn_recovery), GFP_KERNEL);
+   cr = kzalloc(sizeof(*cr), GFP_KERNEL);
if (!cr)
return -1;
 
diff --git a/drivers/target/iscsi/iscsi_target_login.c 
b/drivers/target/iscsi/iscsi_target_login.c
index cfaf564825e0..095651e48f36 100644
--- a/drivers/target/iscsi/iscsi_target_login.c
+++ b/drivers/target/iscsi/iscsi_target_login.c
@@ -46,7 +46,7 @@ static struct iscsi_login *iscsi_login_init_conn(struct 
iscsi_conn *conn)
 {
struct iscsi_login *login;
 
-   login = kzalloc(sizeof(struct iscsi_login), GFP_KERNEL);
+   login = kzalloc(sizeof(*login), GFP_KERNEL);
if (!login)
return NULL;
 
@@ -294,7 +294,7 @@ static int iscsi_login_zero_tsih_s1(
struct iscsi_login_req *pdu = (struct iscsi_login_req *)buf;
int ret;
 
-   sess = kzalloc(sizeof(struct iscsi_session), GFP_KERNEL);
+   sess = kzalloc(sizeof(*sess), GFP_KERNEL);
if (!sess) {
iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_TARGET_ERR,
ISCSI_LOGIN_STATUS_NO_RESOURCES);
@@ -1244,7 +1244,7 @@ static int __iscsi_target_login_thread(struct iscsi_np 
*np)
}
spin_unlock_bh(>np_thread_lock);
 
-   conn = kzalloc(sizeof(struct iscsi_conn), GFP_KERNEL);
+   conn = kzalloc(sizeof(*conn), GFP_KERNEL);
if (!conn) {
/* Get another socket */
return 1;
diff --git a/drivers/target/iscsi/iscsi_target_parameters.c 
b/drivers/target/iscsi/iscsi_target_parameters.c
index a49b0b2a4f6c..110e81a0c97d 100644
--- a/drivers/target/iscsi/iscsi_target_parameters.c
+++ b/drivers/target/iscsi/iscsi_target_parameters.c
@@ -129,7 +129,7 @@ static struct iscsi_param *iscsi_set_default_param(struct 
iscsi_param_list *para
 {
struct iscsi_param *param;
 
-   param = kzalloc(sizeof(struct iscsi_param), GFP_KERNEL);
+   param = kzalloc(sizeof(*param), GFP_KERNEL);
if (!param)
goto out;
 
@@ -199,7 +199,7 @@ int iscsi_create_default_params(struct iscsi_param_list 
**param_list_ptr)
struct iscsi_param *param;
struct iscsi_param_list *pl;
 
-   pl = kzalloc(sizeof(struct iscsi_param_list), GFP_KERNEL);
+   pl = kzalloc(sizeof(*pl), GFP_KERNEL);
if (!pl)
return -ENOMEM;
 
@@ -568,7 +568,7 @@ int iscsi_copy_param_list(
struct iscsi_param *new_param = NULL;
struct iscsi_param_list *param_list;
 
-   param_list = kzalloc(sizeof(struct iscsi_param_list), GFP_KERNEL);
+   param_list = kzalloc(sizeof(*param_list), GFP_KERNEL);
if (!param_list)
return -ENOMEM;
 
@@ -583,7 +583,7 @@ int iscsi_copy_param_list(
continue;
}
 
-   new_param = kzalloc(sizeof(struct iscsi_param), GFP_KERNEL);
+   new_param = kzalloc(sizeof(*new_param), GFP_KERNEL);
if (!new_param)
goto err_out;
 
@@ -714,7 +714,7 @@ static int iscsi_add_notunderstood_response(
return -1;
}
 
-   extra_response = kzalloc(sizeof(struct iscsi_extra_response), 
GFP_KERNEL);
+   extra_response = kzalloc(sizeof(*extra_response), GFP_KERNEL);
if (!extra_response)
return -ENOMEM;
 
diff --git a/drivers/target/iscsi/iscsi_target_seq_pdu_list.c 
b/drivers/target/iscsi/iscsi_target_seq_pdu_list.c
index 3a6e619bb30e..0fd914d92f4f 100644
--- a/drivers/target/iscsi/iscsi_target_seq_pdu_list.c
+++ b/drivers/target/iscsi/iscsi_target_seq_pdu_list.c
@@ -137,7 +137,8 @@ static int iscsit_randomize_pdu_lists(
seq_count++;
   

[PATCH 7/8] target/iscsi: Delete an unnecessary variable initialisation in iscsi_set_default_param()

[SF Markus Elfring]

From: Markus Elfring 
Date: Tue, 12 Dec 2017 21:21:31 +0100

The local variable "param" will be reassigned by a following statement.
Thus omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring 
---
 drivers/target/iscsi/iscsi_target_parameters.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/target/iscsi/iscsi_target_parameters.c 
b/drivers/target/iscsi/iscsi_target_parameters.c
index 151269b8816d..a49b0b2a4f6c 100644
--- a/drivers/target/iscsi/iscsi_target_parameters.c
+++ b/drivers/target/iscsi/iscsi_target_parameters.c
@@ -127,7 +127,7 @@ static struct iscsi_param *iscsi_set_default_param(struct 
iscsi_param_list *para
char *name, char *value, u8 phase, u8 scope, u8 sender,
u16 type_range, u8 use)
 {
-   struct iscsi_param *param = NULL;
+   struct iscsi_param *param;
 
param = kzalloc(sizeof(struct iscsi_param), GFP_KERNEL);
if (!param)
-- 
2.15.1

[PATCH 6/8] target/iscsi: Delete an unnecessary variable initialisation in iscsi_create_default_params()

[SF Markus Elfring]

From: Markus Elfring 
Date: Tue, 12 Dec 2017 21:18:39 +0100

The variable "param" will eventually be set to an appropriate pointer
a bit later. Thus omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring 
---
 drivers/target/iscsi/iscsi_target_parameters.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/target/iscsi/iscsi_target_parameters.c 
b/drivers/target/iscsi/iscsi_target_parameters.c
index 25a3a9550488..151269b8816d 100644
--- a/drivers/target/iscsi/iscsi_target_parameters.c
+++ b/drivers/target/iscsi/iscsi_target_parameters.c
@@ -196,7 +196,7 @@ static struct iscsi_param *iscsi_set_default_param(struct 
iscsi_param_list *para
 /* #warning Add extension keys */
 int iscsi_create_default_params(struct iscsi_param_list **param_list_ptr)
 {
-   struct iscsi_param *param = NULL;
+   struct iscsi_param *param;
struct iscsi_param_list *pl;
 
pl = kzalloc(sizeof(struct iscsi_param_list), GFP_KERNEL);
-- 
2.15.1

[PATCH 5/8] target/iscsi: Delete an unnecessary variable initialisation in iscsi_copy_param_list()

[SF Markus Elfring]

From: Markus Elfring 
Date: Tue, 12 Dec 2017 21:15:55 +0100

The variable "param_list" will be reassigned by a following statement.
Thus omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring 
---
 drivers/target/iscsi/iscsi_target_parameters.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/target/iscsi/iscsi_target_parameters.c 
b/drivers/target/iscsi/iscsi_target_parameters.c
index 06310b2c4e26..25a3a9550488 100644
--- a/drivers/target/iscsi/iscsi_target_parameters.c
+++ b/drivers/target/iscsi/iscsi_target_parameters.c
@@ -566,7 +566,7 @@ int iscsi_copy_param_list(
 {
struct iscsi_param *param = NULL;
struct iscsi_param *new_param = NULL;
-   struct iscsi_param_list *param_list = NULL;
+   struct iscsi_param_list *param_list;
 
param_list = kzalloc(sizeof(struct iscsi_param_list), GFP_KERNEL);
if (!param_list)
-- 
2.15.1

[PATCH 4/8] target/iscsi: Delete an unnecessary variable initialisation in iscsit_allocate_ooo_cmdsn()

[SF Markus Elfring]

From: Markus Elfring 
Date: Tue, 12 Dec 2017 21:13:49 +0100

The local variable "ooo_cmdsn" will be reassigned by a following statement.
Thus omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring 
---
 drivers/target/iscsi/iscsi_target_erl1.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/target/iscsi/iscsi_target_erl1.c 
b/drivers/target/iscsi/iscsi_target_erl1.c
index ff3e08b6d4e1..c4c550128161 100644
--- a/drivers/target/iscsi/iscsi_target_erl1.c
+++ b/drivers/target/iscsi/iscsi_target_erl1.c
@@ -782,7 +782,7 @@ int iscsit_recover_dataout_sequence(
 
 static struct iscsi_ooo_cmdsn *iscsit_allocate_ooo_cmdsn(void)
 {
-   struct iscsi_ooo_cmdsn *ooo_cmdsn = NULL;
+   struct iscsi_ooo_cmdsn *ooo_cmdsn;
 
ooo_cmdsn = kmem_cache_zalloc(lio_ooo_cache, GFP_ATOMIC);
if (!ooo_cmdsn)
-- 
2.15.1

[PATCH 3/8] target/iscsi: Delete 36 error messages for a failed memory allocation

[SF Markus Elfring]

From: Markus Elfring 
Date: Tue, 12 Dec 2017 21:07:16 +0100

Omit extra messages for a memory allocation failure in these functions.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/target/iscsi/iscsi_target.c   |  2 --
 drivers/target/iscsi/iscsi_target_auth.c  | 17 +++--
 drivers/target/iscsi/iscsi_target_datain_values.c |  6 ++--
 drivers/target/iscsi/iscsi_target_erl1.c  | 12 +++
 drivers/target/iscsi/iscsi_target_erl2.c  |  6 ++--
 drivers/target/iscsi/iscsi_target_login.c | 23 +++--
 drivers/target/iscsi/iscsi_target_nego.c  |  4 +--
 drivers/target/iscsi/iscsi_target_parameters.c| 42 +++
 drivers/target/iscsi/iscsi_target_seq_pdu_list.c  | 23 +
 drivers/target/iscsi/iscsi_target_tpg.c   |  9 ++---
 drivers/target/iscsi/iscsi_target_util.c  | 11 +++---
 11 files changed, 46 insertions(+), 109 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target.c 
b/drivers/target/iscsi/iscsi_target.c
index 9eb10d34682c..f3c6ea556ea8 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -812,7 +812,6 @@ int iscsit_add_reject(
 
cmd->buf_ptr = kmemdup(buf, ISCSI_HDR_LEN, GFP_KERNEL);
if (!cmd->buf_ptr) {
-   pr_err("Unable to allocate memory for cmd->buf_ptr\n");
iscsit_free_cmd(cmd, false);
return -1;
}
@@ -849,7 +848,6 @@ static int iscsit_add_reject_from_cmd(
 
cmd->buf_ptr = kmemdup(buf, ISCSI_HDR_LEN, GFP_KERNEL);
if (!cmd->buf_ptr) {
-   pr_err("Unable to allocate memory for cmd->buf_ptr\n");
iscsit_free_cmd(cmd, false);
return -1;
}
diff --git a/drivers/target/iscsi/iscsi_target_auth.c 
b/drivers/target/iscsi/iscsi_target_auth.c
index d837fcbdbaf2..3a17343f43ed 100644
--- a/drivers/target/iscsi/iscsi_target_auth.c
+++ b/drivers/target/iscsi/iscsi_target_auth.c
@@ -80,10 +80,9 @@ static int chap_check_algorithm(const char *a_str)
char *tmp, *orig, *token;
 
tmp = kstrdup(a_str, GFP_KERNEL);
-   if (!tmp) {
-   pr_err("Memory allocation failed for CHAP_A temporary 
buffer\n");
+   if (!tmp)
return CHAP_DIGEST_UNKNOWN;
-   }
+
orig = tmp;
 
token = strsep(, "=");
@@ -198,16 +197,12 @@ static int chap_server_compute_md5(
int auth_ret = -1, ret, challenge_len;
 
challenge = kzalloc(CHAP_CHALLENGE_STR_LEN, GFP_KERNEL);
-   if (!challenge) {
-   pr_err("Unable to allocate challenge buffer\n");
+   if (!challenge)
goto exit;
-   }
 
challenge_binhex = kzalloc(CHAP_CHALLENGE_STR_LEN, GFP_KERNEL);
-   if (!challenge_binhex) {
-   pr_err("Unable to allocate challenge_binhex buffer\n");
+   if (!challenge_binhex)
goto free_challenge;
-   }
 
memset(chap_n, 0, MAX_CHAP_N_SIZE);
 
@@ -257,10 +252,8 @@ static int chap_server_compute_md5(
}
 
desc = kmalloc(sizeof(*desc) + crypto_shash_descsize(tfm), GFP_KERNEL);
-   if (!desc) {
-   pr_err("Unable to allocate struct shash_desc\n");
+   if (!desc)
goto free_shash;
-   }
 
desc->tfm = tfm;
desc->flags = 0;
diff --git a/drivers/target/iscsi/iscsi_target_datain_values.c 
b/drivers/target/iscsi/iscsi_target_datain_values.c
index 173ddd93c757..c591165f9b1b 100644
--- a/drivers/target/iscsi/iscsi_target_datain_values.c
+++ b/drivers/target/iscsi/iscsi_target_datain_values.c
@@ -30,11 +30,9 @@ struct iscsi_datain_req *iscsit_allocate_datain_req(void)
struct iscsi_datain_req *dr;
 
dr = kmem_cache_zalloc(lio_dr_cache, GFP_ATOMIC);
-   if (!dr) {
-   pr_err("Unable to allocate memory for"
-   " struct iscsi_datain_req\n");
+   if (!dr)
return NULL;
-   }
+
INIT_LIST_HEAD(>cmd_datain_node);
 
return dr;
diff --git a/drivers/target/iscsi/iscsi_target_erl1.c 
b/drivers/target/iscsi/iscsi_target_erl1.c
index 5efa42b939a1..ff3e08b6d4e1 100644
--- a/drivers/target/iscsi/iscsi_target_erl1.c
+++ b/drivers/target/iscsi/iscsi_target_erl1.c
@@ -59,11 +59,9 @@ int iscsit_dump_data_payload(
length = min(buf_len, OFFLOAD_BUF_SIZE);
 
buf = kzalloc(length, GFP_ATOMIC);
-   if (!buf) {
-   pr_err("Unable to allocate %u bytes for offload"
-   " buffer.\n", length);
+   if (!buf)
return -1;
-   }
+
memset(, 0, sizeof(struct kvec));
 
while (offset < buf_len) {
@@ -787,11 +785,9 @@ static struct iscsi_ooo_cmdsn 
*iscsit_allocate_ooo_cmdsn(void)
struct iscsi_ooo_cmdsn *ooo_cmdsn = NULL;
 
ooo_cmdsn = 

[PATCH 2/8] target/iscsi: Move resetting of seven variables in chap_server_compute_md5()

[SF Markus Elfring]

From: Markus Elfring 
Date: Tue, 12 Dec 2017 19:43:47 +0100

Move the resetting of these array variables so that this operation will
be performed only if memory allocations succeeded before in this function.

Signed-off-by: Markus Elfring 
---
 drivers/target/iscsi/iscsi_target_auth.c | 22 ++
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target_auth.c 
b/drivers/target/iscsi/iscsi_target_auth.c
index 94b011fe74e8..d837fcbdbaf2 100644
--- a/drivers/target/iscsi/iscsi_target_auth.c
+++ b/drivers/target/iscsi/iscsi_target_auth.c
@@ -197,14 +197,6 @@ static int chap_server_compute_md5(
struct shash_desc *desc;
int auth_ret = -1, ret, challenge_len;
 
-   memset(identifier, 0, 10);
-   memset(chap_n, 0, MAX_CHAP_N_SIZE);
-   memset(chap_r, 0, MAX_RESPONSE_LENGTH);
-   memset(digest, 0, MD5_SIGNATURE_SIZE);
-   memset(response, 0, MD5_SIGNATURE_SIZE * 2 + 2);
-   memset(client_digest, 0, MD5_SIGNATURE_SIZE);
-   memset(server_digest, 0, MD5_SIGNATURE_SIZE);
-
challenge = kzalloc(CHAP_CHALLENGE_STR_LEN, GFP_KERNEL);
if (!challenge) {
pr_err("Unable to allocate challenge buffer\n");
@@ -216,6 +208,9 @@ static int chap_server_compute_md5(
pr_err("Unable to allocate challenge_binhex buffer\n");
goto free_challenge;
}
+
+   memset(chap_n, 0, MAX_CHAP_N_SIZE);
+
/*
 * Extract CHAP_N.
 */
@@ -236,6 +231,8 @@ static int chap_server_compute_md5(
goto free_challenge_binhex;
}
pr_debug("[server] Got CHAP_N=%s\n", chap_n);
+   memset(chap_r, 0, MAX_RESPONSE_LENGTH);
+
/*
 * Extract CHAP_R.
 */
@@ -250,6 +247,7 @@ static int chap_server_compute_md5(
}
 
pr_debug("[server] Got CHAP_R=%s\n", chap_r);
+   memset(client_digest, 0, MD5_SIGNATURE_SIZE);
chap_string_to_hex(client_digest, chap_r, strlen(chap_r));
 
tfm = crypto_alloc_shash("md5", 0, 0);
@@ -286,6 +284,7 @@ static int chap_server_compute_md5(
goto free_desc;
}
 
+   memset(server_digest, 0, MD5_SIGNATURE_SIZE);
ret = crypto_shash_finup(desc, chap->challenge,
 CHAP_CHALLENGE_LENGTH, server_digest);
if (ret < 0) {
@@ -293,6 +292,7 @@ static int chap_server_compute_md5(
goto free_desc;
}
 
+   memset(response, 0, MD5_SIGNATURE_SIZE * 2 + 2);
chap_binaryhex_to_asciihex(response, server_digest, MD5_SIGNATURE_SIZE);
pr_debug("[server] MD5 Server Digest: %s\n", response);
 
@@ -310,6 +310,9 @@ static int chap_server_compute_md5(
auth_ret = 0;
goto free_desc;
}
+
+   memset(identifier, 0, ARRAY_SIZE(identifier));
+
/*
 * Get CHAP_I.
 */
@@ -393,6 +396,9 @@ static int chap_server_compute_md5(
" password_mutual\n");
goto free_desc;
}
+
+   memset(digest, 0, MD5_SIGNATURE_SIZE);
+
/*
 * Convert received challenge to binary hex.
 */
-- 
2.15.1

[PATCH 1/8] target/iscsi: Less function calls in chap_server_compute_md5() after error detection

[SF Markus Elfring]

From: Markus Elfring 
Date: Tue, 12 Dec 2017 18:00:41 +0100

The functions "crypto_free_shash", "kfree" and "kzfree" were called
in a few cases by the chap_server_compute_md5() function during error
handling even if the passed variable contained a null pointer.

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

* Delete initialisations for the variables "challenge", "challenge_binhex",
  "desc" and "tfm" at the beginning which became unnecessary
  with this refactoring.

Fixes: 69110e3cedbb8aad1c70d91ed58a9f4f0ed9eec6 ("iscsi-target: Use shash and 
ahash")
Fixes: e48354ce078c079996f89d715dfa44814b4eba01 ("iscsi-target: Add iSCSI 
fabric support for target v4.1")

Signed-off-by: Markus Elfring 
---
 drivers/target/iscsi/iscsi_target_auth.c | 71 +---
 1 file changed, 37 insertions(+), 34 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target_auth.c 
b/drivers/target/iscsi/iscsi_target_auth.c
index f9bc8ec6fb6b..94b011fe74e8 100644
--- a/drivers/target/iscsi/iscsi_target_auth.c
+++ b/drivers/target/iscsi/iscsi_target_auth.c
@@ -186,15 +186,15 @@ static int chap_server_compute_md5(
unsigned char id_as_uchar;
unsigned char digest[MD5_SIGNATURE_SIZE];
unsigned char type, response[MD5_SIGNATURE_SIZE * 2 + 2];
-   unsigned char identifier[10], *challenge = NULL;
-   unsigned char *challenge_binhex = NULL;
+   unsigned char identifier[10], *challenge;
+   unsigned char *challenge_binhex;
unsigned char client_digest[MD5_SIGNATURE_SIZE];
unsigned char server_digest[MD5_SIGNATURE_SIZE];
unsigned char chap_n[MAX_CHAP_N_SIZE], chap_r[MAX_RESPONSE_LENGTH];
size_t compare_len;
struct iscsi_chap *chap = conn->auth_protocol;
-   struct crypto_shash *tfm = NULL;
-   struct shash_desc *desc = NULL;
+   struct crypto_shash *tfm;
+   struct shash_desc *desc;
int auth_ret = -1, ret, challenge_len;
 
memset(identifier, 0, 10);
@@ -208,13 +208,13 @@ static int chap_server_compute_md5(
challenge = kzalloc(CHAP_CHALLENGE_STR_LEN, GFP_KERNEL);
if (!challenge) {
pr_err("Unable to allocate challenge buffer\n");
-   goto out;
+   goto exit;
}
 
challenge_binhex = kzalloc(CHAP_CHALLENGE_STR_LEN, GFP_KERNEL);
if (!challenge_binhex) {
pr_err("Unable to allocate challenge_binhex buffer\n");
-   goto out;
+   goto free_challenge;
}
/*
 * Extract CHAP_N.
@@ -222,18 +222,18 @@ static int chap_server_compute_md5(
if (extract_param(nr_in_ptr, "CHAP_N", MAX_CHAP_N_SIZE, chap_n,
) < 0) {
pr_err("Could not find CHAP_N.\n");
-   goto out;
+   goto free_challenge_binhex;
}
if (type == HEX) {
pr_err("Could not find CHAP_N.\n");
-   goto out;
+   goto free_challenge_binhex;
}
 
/* Include the terminating NULL in the compare */
compare_len = strlen(auth->userid) + 1;
if (strncmp(chap_n, auth->userid, compare_len) != 0) {
pr_err("CHAP_N values do not match!\n");
-   goto out;
+   goto free_challenge_binhex;
}
pr_debug("[server] Got CHAP_N=%s\n", chap_n);
/*
@@ -242,11 +242,11 @@ static int chap_server_compute_md5(
if (extract_param(nr_in_ptr, "CHAP_R", MAX_RESPONSE_LENGTH, chap_r,
) < 0) {
pr_err("Could not find CHAP_R.\n");
-   goto out;
+   goto free_challenge_binhex;
}
if (type != HEX) {
pr_err("Could not find CHAP_R.\n");
-   goto out;
+   goto free_challenge_binhex;
}
 
pr_debug("[server] Got CHAP_R=%s\n", chap_r);
@@ -254,15 +254,14 @@ static int chap_server_compute_md5(
 
tfm = crypto_alloc_shash("md5", 0, 0);
if (IS_ERR(tfm)) {
-   tfm = NULL;
pr_err("Unable to allocate struct crypto_shash\n");
-   goto out;
+   goto free_challenge_binhex;
}
 
desc = kmalloc(sizeof(*desc) + crypto_shash_descsize(tfm), GFP_KERNEL);
if (!desc) {
pr_err("Unable to allocate struct shash_desc\n");
-   goto out;
+   goto free_shash;
}
 
desc->tfm = tfm;
@@ -271,27 +270,27 @@ static int chap_server_compute_md5(
ret = crypto_shash_init(desc);
if (ret < 0) {
pr_err("crypto_shash_init() failed\n");
-   goto out;
+   goto free_desc;
}
 
ret = crypto_shash_update(desc, >id, 1);
if (ret < 0) {
pr_err("crypto_shash_update() failed for id\n");
-   goto out;
+   goto 

[PATCH 0/8] target-iSCSI: Adjustments for several function implementations

[SF Markus Elfring]

From: Markus Elfring 
Date: Tue, 12 Dec 2017 22:22:11 +0100

Some update suggestions were taken into account
from static source code analysis.

Markus Elfring (8):
  Less function calls in chap_server_compute_md5() after error detection
  Move resetting of seven variables in chap_server_compute_md5()
  Delete 36 error messages for a failed memory allocation
  Delete an unnecessary variable initialisation in iscsit_allocate_ooo_cmdsn()
  Delete an unnecessary variable initialisation in iscsi_copy_param_list()
  Delete an unnecessary variable initialisation in iscsi_create_default_params()
  Delete an unnecessary variable initialisation in iscsi_set_default_param()
  Improve 16 size determinations

 drivers/target/iscsi/iscsi_target.c   |   2 -
 drivers/target/iscsi/iscsi_target_auth.c  | 110 +++---
 drivers/target/iscsi/iscsi_target_datain_values.c |   6 +-
 drivers/target/iscsi/iscsi_target_erl1.c  |  14 +--
 drivers/target/iscsi/iscsi_target_erl2.c  |   8 +-
 drivers/target/iscsi/iscsi_target_login.c |  29 ++
 drivers/target/iscsi/iscsi_target_nego.c  |   4 +-
 drivers/target/iscsi/iscsi_target_parameters.c|  58 +---
 drivers/target/iscsi/iscsi_target_seq_pdu_list.c  |  34 +++
 drivers/target/iscsi/iscsi_target_tpg.c   |  13 +--
 drivers/target/iscsi/iscsi_target_util.c  |  11 +--
 11 files changed, 118 insertions(+), 171 deletions(-)

-- 
2.15.1

[PATCH 2/2] target: cxgbit: Combine substrings for 11 messages

[SF Markus Elfring]

From: Markus Elfring 
Date: Tue, 12 Dec 2017 12:57:47 +0100

The script "checkpatch.pl" pointed information out like the following.

WARNING: quoted string split across lines

Thus fix the affected source code places.

Signed-off-by: Markus Elfring 
---
 drivers/target/iscsi/cxgbit/cxgbit_target.c | 50 +
 1 file changed, 16 insertions(+), 34 deletions(-)

diff --git a/drivers/target/iscsi/cxgbit/cxgbit_target.c 
b/drivers/target/iscsi/cxgbit/cxgbit_target.c
index 9163152b6d0b..e4a0dbc8fb9d 100644
--- a/drivers/target/iscsi/cxgbit/cxgbit_target.c
+++ b/drivers/target/iscsi/cxgbit/cxgbit_target.c
@@ -881,9 +881,7 @@ cxgbit_handle_immediate_data(struct iscsi_cmd *cmd, struct 
iscsi_scsi_req *hdr,
if (pdu_cb->flags & PDUCBF_RX_DCRC_ERR) {
pr_err("ImmediateData CRC32C DataDigest error\n");
if (!conn->sess->sess_ops->ErrorRecoveryLevel) {
-   pr_err("Unable to recover from"
-  " Immediate Data digest failure while"
-  " in ERL=0.\n");
+   pr_err("Unable to recover from Immediate Data digest 
failure while in ERL=0.\n");
iscsit_reject_cmd(cmd, ISCSI_REASON_DATA_DIGEST_ERROR,
  (unsigned char *)hdr);
return IMMEDIATE_DATA_CANNOT_RECOVER;
@@ -1054,19 +1052,14 @@ static int cxgbit_handle_iscsi_dataout(struct 
cxgbit_sock *csk)
}
 
if (pdu_cb->flags & PDUCBF_RX_DCRC_ERR) {
-   pr_err("ITT: 0x%08x, Offset: %u, Length: %u,"
-  " DataSN: 0x%08x\n",
-  hdr->itt, hdr->offset, data_len,
-  hdr->datasn);
-
+   pr_err("ITT: 0x%08x, Offset: %u, Length: %u, DataSN: 0x%08x\n",
+  hdr->itt, hdr->offset, data_len, hdr->datasn);
dcrc_err = true;
goto check_payload;
}
 
-   pr_debug("DataOut data_len: %u, "
-   "write_data_done: %u, data_length: %u\n",
- data_len,  cmd->write_data_done,
- cmd->se_cmd.data_length);
+   pr_debug("DataOut data_len: %u, write_data_done: %u, data_length: %u\n",
+data_len, cmd->write_data_done, cmd->se_cmd.data_length);
 
if (!(pdu_cb->flags & PDUCBF_RX_DATA_DDPD)) {
u32 skip = data_offset % PAGE_SIZE;
@@ -1102,9 +1095,7 @@ static int cxgbit_handle_nop_out(struct cxgbit_sock *csk, 
struct iscsi_cmd *cmd)
 
if (pdu_cb->flags & PDUCBF_RX_DCRC_ERR) {
if (!conn->sess->sess_ops->ErrorRecoveryLevel) {
-   pr_err("Unable to recover from"
-  " NOPOUT Ping DataCRC failure while in"
-  " ERL=0.\n");
+   pr_err("Unable to recover from NOPOUT Ping DataCRC 
failure while in ERL=0.\n");
ret = -1;
goto out;
} else {
@@ -1112,9 +1103,8 @@ static int cxgbit_handle_nop_out(struct cxgbit_sock *csk, 
struct iscsi_cmd *cmd)
 * drop this PDU and let the
 * initiator plug the CmdSN gap.
 */
-   pr_info("Dropping NOPOUT"
-   " Command CmdSN: 0x%08x due to"
-   " DataCRC error.\n", hdr->cmdsn);
+   pr_info("Dropping NOPOUT Command CmdSN: 0x%08x due to 
DataCRC error.\n",
+   hdr->cmdsn);
ret = 0;
goto out;
}
@@ -1139,9 +1129,7 @@ static int cxgbit_handle_nop_out(struct cxgbit_sock *csk, 
struct iscsi_cmd *cmd)
 */
cmd->buf_ptr = ping_data;
cmd->buf_ptr_size = payload_length;
-
-   pr_debug("Got %u bytes of NOPOUT ping"
-   " data.\n", payload_length);
+   pr_debug("Got %u bytes of NOPOUT ping data.\n", payload_length);
pr_debug("Ping Data: \"%s\"\n", ping_data);
}
 
@@ -1168,18 +1156,15 @@ cxgbit_handle_text_cmd(struct cxgbit_sock *csk, struct 
iscsi_cmd *cmd)
 
if (pdu_cb->flags & PDUCBF_RX_DCRC_ERR) {
if (!conn->sess->sess_ops->ErrorRecoveryLevel) {
-   pr_err("Unable to recover from"
-  " Text Data digest failure while in"
-  " ERL=0.\n");
+   pr_err("Unable to recover from Text Data digest failure 
while in ERL=0.\n");
goto reject;
} else {
/*
 * drop this PDU and let the
 * initiator plug the CmdSN gap.
 */
-   pr_info("Dropping Text"
- 

[PATCH 1/2] target: cxgbit: Delete an error message for a failed memory allocation in two functions

[SF Markus Elfring]

From: Markus Elfring 
Date: Tue, 12 Dec 2017 11:34:51 +0100

Omit an extra message for a memory allocation failure in these functions.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/target/iscsi/cxgbit/cxgbit_target.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/target/iscsi/cxgbit/cxgbit_target.c 
b/drivers/target/iscsi/cxgbit/cxgbit_target.c
index 514986b57c2d..9163152b6d0b 100644
--- a/drivers/target/iscsi/cxgbit/cxgbit_target.c
+++ b/drivers/target/iscsi/cxgbit/cxgbit_target.c
@@ -1126,8 +1126,6 @@ static int cxgbit_handle_nop_out(struct cxgbit_sock *csk, 
struct iscsi_cmd *cmd)
if (payload_length && hdr->ttt == cpu_to_be32(0x)) {
ping_data = kzalloc(payload_length + 1, GFP_KERNEL);
if (!ping_data) {
-   pr_err("Unable to allocate memory for"
-   " NOPOUT ping data.\n");
ret = -1;
goto out;
}
@@ -1188,11 +1186,9 @@ cxgbit_handle_text_cmd(struct cxgbit_sock *csk, struct 
iscsi_cmd *cmd)
 
if (payload_length) {
text_in = kzalloc(payload_length, GFP_KERNEL);
-   if (!text_in) {
-   pr_err("Unable to allocate text_in of payload_length: 
%u\n",
-  payload_length);
+   if (!text_in)
return -ENOMEM;
-   }
+
skb_copy_bits(csk->skb, pdu_cb->doffset,
  text_in, payload_length);
 
-- 
2.15.1

[PATCH 0/2] target/iscsi/cxgbit: Adjustments for seven function implementations

[SF Markus Elfring]

From: Markus Elfring 
Date: Tue, 12 Dec 2017 13:12:11 +0100

Two update suggestions were taken into account
from static source code analysis.

Markus Elfring (2):
  Delete an error message for a failed memory allocation in two functions
  Combine substrings for 11 messages

 drivers/target/iscsi/cxgbit/cxgbit_target.c | 58 +
 1 file changed, 18 insertions(+), 40 deletions(-)

-- 
2.15.1

[PATCH 6/6] target: tcm_loop: Use seq_puts() in tcm_loop_show_info()

[SF Markus Elfring]

From: Markus Elfring 
Date: Mon, 11 Dec 2017 11:01:57 +0100

The script "checkpatch.pl" pointed information out like the following.

WARNING: Prefer seq_puts to seq_printf

Thus fix the affected source code place.

Signed-off-by: Markus Elfring 
---
 drivers/target/loopback/tcm_loop.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/target/loopback/tcm_loop.c 
b/drivers/target/loopback/tcm_loop.c
index ec01070bbb40..9cd4ffe76c07 100644
--- a/drivers/target/loopback/tcm_loop.c
+++ b/drivers/target/loopback/tcm_loop.c
@@ -64,7 +64,7 @@ static void tcm_loop_release_cmd(struct se_cmd *se_cmd)
 
 static int tcm_loop_show_info(struct seq_file *m, struct Scsi_Host *host)
 {
-   seq_printf(m, "tcm_loop_proc_info()\n");
+   seq_puts(m, "tcm_loop_proc_info()\n");
return 0;
 }
 
-- 
2.15.1

[PATCH 5/6] target: tcm_loop: Delete an unnecessary return statement in tcm_loop_submission_work()

[SF Markus Elfring]

From: Markus Elfring 
Date: Mon, 11 Dec 2017 10:58:33 +0100

The script "checkpatch.pl" pointed information out like the following.

WARNING: void function return statements are not generally useful

Thus remove such a statement in the affected function.

Signed-off-by: Markus Elfring 
---
 drivers/target/loopback/tcm_loop.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/target/loopback/tcm_loop.c 
b/drivers/target/loopback/tcm_loop.c
index 08dbf5b4ff15..ec01070bbb40 100644
--- a/drivers/target/loopback/tcm_loop.c
+++ b/drivers/target/loopback/tcm_loop.c
@@ -166,7 +166,6 @@ static void tcm_loop_submission_work(struct work_struct 
*work)
 out_done:
kmem_cache_free(tcm_loop_cmd_cache, tl_cmd);
sc->scsi_done(sc);
-   return;
 }
 
 /*
-- 
2.15.1

[PATCH 4/6] target: tcm_loop: Delete two unnecessary variable initialisations in tcm_loop_issue_tmr()

[SF Markus Elfring]

From: Markus Elfring 
Date: Mon, 11 Dec 2017 10:56:42 +0100

The variables "se_cmd" and "tl_cmd" will eventually be set to appropriate
pointers a bit later.
Thus omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring 
---
 drivers/target/loopback/tcm_loop.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/target/loopback/tcm_loop.c 
b/drivers/target/loopback/tcm_loop.c
index d91109c1c4ae..08dbf5b4ff15 100644
--- a/drivers/target/loopback/tcm_loop.c
+++ b/drivers/target/loopback/tcm_loop.c
@@ -203,10 +203,10 @@ static int tcm_loop_queuecommand(struct Scsi_Host *sh, 
struct scsi_cmnd *sc)
 static int tcm_loop_issue_tmr(struct tcm_loop_tpg *tl_tpg,
  u64 lun, int task, enum tcm_tmreq_table tmr)
 {
-   struct se_cmd *se_cmd = NULL;
+   struct se_cmd *se_cmd;
struct se_session *se_sess;
struct tcm_loop_nexus *tl_nexus;
-   struct tcm_loop_cmd *tl_cmd = NULL;
+   struct tcm_loop_cmd *tl_cmd;
int ret = TMR_FUNCTION_FAILED, rc;
 
/*
-- 
2.15.1

[PATCH 3/6] target: tcm_loop: Combine substrings for 26 messages

[SF Markus Elfring]

From: Markus Elfring 
Date: Mon, 11 Dec 2017 10:40:23 +0100

The script "checkpatch.pl" pointed information out like the following.

WARNING: quoted string split across lines

Thus fix the affected source code places.

Signed-off-by: Markus Elfring 
---
 drivers/target/loopback/tcm_loop.c | 120 -
 1 file changed, 52 insertions(+), 68 deletions(-)

diff --git a/drivers/target/loopback/tcm_loop.c 
b/drivers/target/loopback/tcm_loop.c
index 60e4185e7508..d91109c1c4ae 100644
--- a/drivers/target/loopback/tcm_loop.c
+++ b/drivers/target/loopback/tcm_loop.c
@@ -123,8 +123,8 @@ static void tcm_loop_submission_work(struct work_struct 
*work)
}
tl_nexus = tl_tpg->tl_nexus;
if (!tl_nexus) {
-   scmd_printk(KERN_ERR, sc, "TCM_Loop I_T Nexus"
-   " does not exist\n");
+   scmd_printk(KERN_ERR, sc,
+   "TCM_Loop I_T Nexus does not exist\n");
set_host_byte(sc, DID_ERROR);
goto out_done;
}
@@ -177,10 +177,10 @@ static int tcm_loop_queuecommand(struct Scsi_Host *sh, 
struct scsi_cmnd *sc)
 {
struct tcm_loop_cmd *tl_cmd;
 
-   pr_debug("tcm_loop_queuecommand() %d:%d:%d:%llu got CDB: 0x%02x"
-   " scsi_buf_len: %u\n", sc->device->host->host_no,
-   sc->device->id, sc->device->channel, sc->device->lun,
-   sc->cmnd[0], scsi_bufflen(sc));
+   pr_debug("%s() %d:%d:%d:%llu got CDB: 0x%02x scsi_buf_len: %u\n",
+__func__, sc->device->host->host_no, sc->device->id,
+sc->device->channel, sc->device->lun, sc->cmnd[0],
+scsi_bufflen(sc));
 
tl_cmd = kmem_cache_zalloc(tcm_loop_cmd_cache, GFP_ATOMIC);
if (!tl_cmd) {
@@ -214,8 +214,7 @@ static int tcm_loop_issue_tmr(struct tcm_loop_tpg *tl_tpg,
 */
tl_nexus = tl_tpg->tl_nexus;
if (!tl_nexus) {
-   pr_err("Unable to perform device reset without"
-   " active I_T Nexus\n");
+   pr_err("Unable to perform device reset without active I_T 
Nexus\n");
return ret;
}
 
@@ -295,8 +294,7 @@ static int tcm_loop_target_reset(struct scsi_cmnd *sc)
 */
tl_hba = *(struct tcm_loop_hba **)shost_priv(sc->device->host);
if (!tl_hba) {
-   pr_err("Unable to perform device reset without"
-   " active I_T Nexus\n");
+   pr_err("Unable to perform device reset without active I_T 
Nexus\n");
return FAILED;
}
/*
@@ -414,8 +412,7 @@ static int tcm_loop_setup_hba_bus(struct tcm_loop_hba 
*tl_hba, int tcm_loop_host
 
ret = device_register(_hba->dev);
if (ret) {
-   pr_err("device_register() failed for"
-   " tl_hba->dev: %d\n", ret);
+   pr_err("device_register() failed for tl_hba->dev: %d\n", ret);
return -ENODEV;
}
 
@@ -444,8 +441,7 @@ static int tcm_loop_alloc_core_bus(void)
 
ret = driver_register(_loop_driverfs);
if (ret) {
-   pr_err("driver_register() failed for"
-   "tcm_loop_driverfs\n");
+   pr_err("driver_register() failed for tcm_loop_driverfs\n");
goto bus_unreg;
}
 
@@ -584,8 +580,8 @@ static int tcm_loop_queue_data_in(struct se_cmd *se_cmd)
struct tcm_loop_cmd, tl_se_cmd);
struct scsi_cmnd *sc = tl_cmd->sc;
 
-   pr_debug("tcm_loop_queue_data_in() called for scsi_cmnd: %p"
-" cdb: 0x%02x\n", sc, sc->cmnd[0]);
+   pr_debug("%s() called for scsi_cmnd: %p cdb: 0x%02x\n",
+__func__, sc, sc->cmnd[0]);
 
sc->result = SAM_STAT_GOOD;
set_host_byte(sc, DID_OK);
@@ -602,8 +598,8 @@ static int tcm_loop_queue_status(struct se_cmd *se_cmd)
struct tcm_loop_cmd, tl_se_cmd);
struct scsi_cmnd *sc = tl_cmd->sc;
 
-   pr_debug("tcm_loop_queue_status() called for scsi_cmnd: %p"
-   " cdb: 0x%02x\n", sc, sc->cmnd[0]);
+   pr_debug("%s() called for scsi_cmnd: %p cdb: 0x%02x\n",
+__func__, sc, sc->cmnd[0]);
 
if (se_cmd->sense_buffer &&
   ((se_cmd->se_cmd_flags & SCF_TRANSPORT_TASK_SENSE) ||
@@ -688,8 +684,8 @@ static void tcm_loop_port_unlink(
sd = scsi_device_lookup(tl_hba->sh, 0, tl_tpg->tl_tpgt,
se_lun->unpacked_lun);
if (!sd) {
-   pr_err("Unable to locate struct scsi_device for %d:%d:"
-   "%llu\n", 0, tl_tpg->tl_tpgt, se_lun->unpacked_lun);
+   pr_err("Unable to locate struct scsi_device for %d:%d:%llu\n",
+  0, tl_tpg->tl_tpgt, se_lun->unpacked_lun);
return;
  

[PATCH 2/6] target: tcm_loop: Improve a size determination in two functions

[SF Markus Elfring]

From: Markus Elfring 
Date: Sun, 10 Dec 2017 21:23:43 +0100

Replace the specification of data structures by pointer dereferences
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/target/loopback/tcm_loop.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/target/loopback/tcm_loop.c 
b/drivers/target/loopback/tcm_loop.c
index 6bd58a064924..60e4185e7508 100644
--- a/drivers/target/loopback/tcm_loop.c
+++ b/drivers/target/loopback/tcm_loop.c
@@ -769,7 +769,7 @@ static int tcm_loop_make_nexus(
return -EEXIST;
}
 
-   tl_nexus = kzalloc(sizeof(struct tcm_loop_nexus), GFP_KERNEL);
+   tl_nexus = kzalloc(sizeof(*tl_nexus), GFP_KERNEL);
if (!tl_nexus)
return -ENOMEM;
 
@@ -1076,7 +1076,7 @@ static struct se_wwn *tcm_loop_make_scsi_hba(
char *ptr;
int ret, off = 0;
 
-   tl_hba = kzalloc(sizeof(struct tcm_loop_hba), GFP_KERNEL);
+   tl_hba = kzalloc(sizeof(*tl_hba), GFP_KERNEL);
if (!tl_hba)
return ERR_PTR(-ENOMEM);
 
-- 
2.15.1

[PATCH 1/6] target: tcm_loop: Delete an error message for a failed memory allocation in four functions

[SF Markus Elfring]

From: Markus Elfring 
Date: Sun, 10 Dec 2017 21:18:15 +0100

Omit an extra message for a memory allocation failure in these functions.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/target/loopback/tcm_loop.c | 14 --
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/target/loopback/tcm_loop.c 
b/drivers/target/loopback/tcm_loop.c
index b6a913e38b30..6bd58a064924 100644
--- a/drivers/target/loopback/tcm_loop.c
+++ b/drivers/target/loopback/tcm_loop.c
@@ -184,7 +184,6 @@ static int tcm_loop_queuecommand(struct Scsi_Host *sh, 
struct scsi_cmnd *sc)
 
tl_cmd = kmem_cache_zalloc(tcm_loop_cmd_cache, GFP_ATOMIC);
if (!tl_cmd) {
-   pr_err("Unable to allocate struct tcm_loop_cmd\n");
set_host_byte(sc, DID_ERROR);
sc->scsi_done(sc);
return 0;
@@ -221,10 +220,8 @@ static int tcm_loop_issue_tmr(struct tcm_loop_tpg *tl_tpg,
}
 
tl_cmd = kmem_cache_zalloc(tcm_loop_cmd_cache, GFP_KERNEL);
-   if (!tl_cmd) {
-   pr_err("Unable to allocate memory for tl_cmd\n");
+   if (!tl_cmd)
return ret;
-   }
 
init_completion(_cmd->tmr_done);
 
@@ -773,10 +770,8 @@ static int tcm_loop_make_nexus(
}
 
tl_nexus = kzalloc(sizeof(struct tcm_loop_nexus), GFP_KERNEL);
-   if (!tl_nexus) {
-   pr_err("Unable to allocate struct tcm_loop_nexus\n");
+   if (!tl_nexus)
return -ENOMEM;
-   }
 
tl_nexus->se_sess = target_alloc_session(_tpg->tl_se_tpg, 0, 0,
TARGET_PROT_DIN_PASS | 
TARGET_PROT_DOUT_PASS,
@@ -1082,10 +1077,9 @@ static struct se_wwn *tcm_loop_make_scsi_hba(
int ret, off = 0;
 
tl_hba = kzalloc(sizeof(struct tcm_loop_hba), GFP_KERNEL);
-   if (!tl_hba) {
-   pr_err("Unable to allocate struct tcm_loop_hba\n");
+   if (!tl_hba)
return ERR_PTR(-ENOMEM);
-   }
+
/*
 * Determine the emulated Protocol Identifier and Target Port Name
 * based on the incoming configfs directory name.
-- 
2.15.1

[PATCH 0/6] target/loopback/tcm: Adjustments for some function implementations

[SF Markus Elfring]

From: Markus Elfring 
Date: Mon, 11 Dec 2017 11:33:44 +0100

Some update suggestions were taken into account
from static source code analysis.

Markus Elfring (6):
  Delete an error message for a failed memory allocation in four functions
  Improve a size determination in two functions
  Combine substrings for 26 messages
  Delete two unnecessary variable initialisations in tcm_loop_issue_tmr()
  Delete an unnecessary return statement in tcm_loop_submission_work()
  Use seq_puts() in tcm_loop_show_info()

 drivers/target/loopback/tcm_loop.c | 145 -
 1 file changed, 61 insertions(+), 84 deletions(-)

-- 
2.15.1

[PATCH] sbp-target: Delete an error message for a failed memory allocation in three functions

[SF Markus Elfring]

From: Markus Elfring 
Date: Sun, 10 Dec 2017 19:54:11 +0100

Omit an extra message for a memory allocation failure in these functions.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/target/sbp/sbp_target.c | 13 -
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/target/sbp/sbp_target.c b/drivers/target/sbp/sbp_target.c
index e5c3e5f827d0..fb1003921d85 100644
--- a/drivers/target/sbp/sbp_target.c
+++ b/drivers/target/sbp/sbp_target.c
@@ -201,10 +201,9 @@ static struct sbp_session *sbp_session_create(
snprintf(guid_str, sizeof(guid_str), "%016llx", guid);
 
sess = kmalloc(sizeof(*sess), GFP_KERNEL);
-   if (!sess) {
-   pr_err("failed to allocate session descriptor\n");
+   if (!sess)
return ERR_PTR(-ENOMEM);
-   }
+
spin_lock_init(>lock);
INIT_LIST_HEAD(>login_list);
INIT_DELAYED_WORK(>maint_work, session_maintenance_work);
@@ -2029,10 +2028,8 @@ static struct se_portal_group *sbp_make_tpg(
}
 
tpg = kzalloc(sizeof(*tpg), GFP_KERNEL);
-   if (!tpg) {
-   pr_err("Unable to allocate struct sbp_tpg\n");
+   if (!tpg)
return ERR_PTR(-ENOMEM);
-   }
 
tpg->tport = tport;
tpg->tport_tpgt = tpgt;
@@ -2088,10 +2085,8 @@ static struct se_wwn *sbp_make_tport(
return ERR_PTR(-EINVAL);
 
tport = kzalloc(sizeof(*tport), GFP_KERNEL);
-   if (!tport) {
-   pr_err("Unable to allocate struct sbp_tport\n");
+   if (!tport)
return ERR_PTR(-ENOMEM);
-   }
 
tport->guid = guid;
sbp_format_wwn(tport->tport_name, SBP_NAMELEN, guid);
-- 
2.15.1

[PATCH 4/4] target: Delete an error message for a failed memory allocation in two functions

[SF Markus Elfring]

From: Markus Elfring 
Date: Sat, 4 Nov 2017 13:43:22 +0100

Omit an extra message for a memory allocation failure in these functions.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/target/target_core_xcopy.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/target/target_core_xcopy.c 
b/drivers/target/target_core_xcopy.c
index 1519e399392b..c2b3d3158208 100644
--- a/drivers/target/target_core_xcopy.c
+++ b/drivers/target/target_core_xcopy.c
@@ -681,10 +681,9 @@ static int target_xcopy_read_source(
bool remote_port = (xop->op_origin == XCOL_DEST_RECV_OP);
 
xpt_cmd = kzalloc(sizeof(*xpt_cmd), GFP_KERNEL);
-   if (!xpt_cmd) {
-   pr_err("Unable to allocate xcopy_pt_cmd\n");
+   if (!xpt_cmd)
return -ENOMEM;
-   }
+
init_completion(_cmd->xpt_passthrough_sem);
se_cmd = _cmd->se_cmd;
 
@@ -743,10 +742,9 @@ static int target_xcopy_write_destination(
bool remote_port = (xop->op_origin == XCOL_SOURCE_RECV_OP);
 
xpt_cmd = kzalloc(sizeof(*xpt_cmd), GFP_KERNEL);
-   if (!xpt_cmd) {
-   pr_err("Unable to allocate xcopy_pt_cmd\n");
+   if (!xpt_cmd)
return -ENOMEM;
-   }
+
init_completion(_cmd->xpt_passthrough_sem);
se_cmd = _cmd->se_cmd;
 
-- 
2.15.0

[PATCH 3/4] target: Improve a size determination in three functions

[SF Markus Elfring]

From: Markus Elfring 
Date: Sat, 4 Nov 2017 13:35:08 +0100

Replace the specification of data structures by pointer dereferences
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/target/target_core_xcopy.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/target/target_core_xcopy.c 
b/drivers/target/target_core_xcopy.c
index 46cac36014df..1519e399392b 100644
--- a/drivers/target/target_core_xcopy.c
+++ b/drivers/target/target_core_xcopy.c
@@ -680,7 +680,7 @@ static int target_xcopy_read_source(
unsigned char cdb[16];
bool remote_port = (xop->op_origin == XCOL_DEST_RECV_OP);
 
-   xpt_cmd = kzalloc(sizeof(struct xcopy_pt_cmd), GFP_KERNEL);
+   xpt_cmd = kzalloc(sizeof(*xpt_cmd), GFP_KERNEL);
if (!xpt_cmd) {
pr_err("Unable to allocate xcopy_pt_cmd\n");
return -ENOMEM;
@@ -742,7 +742,7 @@ static int target_xcopy_write_destination(
unsigned char cdb[16];
bool remote_port = (xop->op_origin == XCOL_SOURCE_RECV_OP);
 
-   xpt_cmd = kzalloc(sizeof(struct xcopy_pt_cmd), GFP_KERNEL);
+   xpt_cmd = kzalloc(sizeof(*xpt_cmd), GFP_KERNEL);
if (!xpt_cmd) {
pr_err("Unable to allocate xcopy_pt_cmd\n");
return -ENOMEM;
@@ -1010,7 +1010,7 @@ sense_reason_t target_do_xcopy(struct se_cmd *se_cmd)
return TCM_PARAMETER_LIST_LENGTH_ERROR;
}
 
-   xop = kzalloc(sizeof(struct xcopy_op), GFP_KERNEL);
+   xop = kzalloc(sizeof(*xop), GFP_KERNEL);
if (!xop)
goto err;
xop->xop_se_cmd = se_cmd;
-- 
2.15.0

[PATCH 2/4] target: Combine two condition checks into one statement in target_xcopy_do_work()

[SF Markus Elfring]

From: Markus Elfring 
Date: Sat, 4 Nov 2017 13:26:46 +0100

The same label was used by goto statements after two condition checks.
Thus use a single statement instead.

Signed-off-by: Markus Elfring 
---
 drivers/target/target_core_xcopy.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/target/target_core_xcopy.c 
b/drivers/target/target_core_xcopy.c
index b06877c57765..46cac36014df 100644
--- a/drivers/target/target_core_xcopy.c
+++ b/drivers/target/target_core_xcopy.c
@@ -802,10 +802,8 @@ static void target_xcopy_do_work(struct work_struct *work)
int rc = 0;
unsigned short nolb, cur_nolb, max_nolb, copied_nolb = 0;
 
-   if (target_parse_xcopy_cmd(xop) != TCM_NO_SENSE)
-   goto err_free_xop;
-
-   if (WARN_ON_ONCE(!xop->src_dev) || WARN_ON_ONCE(!xop->dst_dev))
+   if (target_parse_xcopy_cmd(xop) != TCM_NO_SENSE ||
+   WARN_ON_ONCE(!xop->src_dev) || WARN_ON_ONCE(!xop->dst_dev))
goto err_free_xop;
 
src_dev = xop->src_dev;
-- 
2.15.0

[PATCH 1/4] target: Use common error handling code in three functions

[SF Markus Elfring]

From: Markus Elfring 
Date: Sat, 4 Nov 2017 13:13:20 +0100

Adjust jump targets so that a bit of exception handling can be better
reused at the end of these functions.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/target/target_core_xcopy.c | 49 +++---
 1 file changed, 25 insertions(+), 24 deletions(-)

diff --git a/drivers/target/target_core_xcopy.c 
b/drivers/target/target_core_xcopy.c
index 9ee89e00cd77..b06877c57765 100644
--- a/drivers/target/target_core_xcopy.c
+++ b/drivers/target/target_core_xcopy.c
@@ -701,11 +701,8 @@ static int target_xcopy_read_source(
 
rc = target_xcopy_setup_pt_cmd(xpt_cmd, xop, src_dev, [0],
remote_port, true);
-   if (rc < 0) {
-   ec_cmd->scsi_status = xpt_cmd->se_cmd.scsi_status;
-   transport_generic_free_cmd(se_cmd, 0);
-   return rc;
-   }
+   if (rc < 0)
+   goto set_status;
 
xop->xop_data_sg = se_cmd->t_data_sg;
xop->xop_data_nents = se_cmd->t_data_nents;
@@ -713,11 +710,9 @@ static int target_xcopy_read_source(
" memory\n", xop->xop_data_sg, xop->xop_data_nents);
 
rc = target_xcopy_issue_pt_cmd(xpt_cmd);
-   if (rc < 0) {
-   ec_cmd->scsi_status = xpt_cmd->se_cmd.scsi_status;
-   transport_generic_free_cmd(se_cmd, 0);
-   return rc;
-   }
+   if (rc < 0)
+   goto set_status;
+
/*
 * Clear off the allocated t_data_sg, that has been saved for
 * zero-copy WRITE submission reuse in struct xcopy_op..
@@ -726,6 +721,11 @@ static int target_xcopy_read_source(
se_cmd->t_data_nents = 0;
 
return 0;
+
+set_status:
+   ec_cmd->scsi_status = xpt_cmd->se_cmd.scsi_status;
+   transport_generic_free_cmd(se_cmd, 0);
+   return rc;
 }
 
 static int target_xcopy_write_destination(
@@ -775,19 +775,21 @@ static int target_xcopy_write_destination(
src_cmd->t_data_sg = xop->xop_data_sg;
src_cmd->t_data_nents = xop->xop_data_nents;
 
-   transport_generic_free_cmd(se_cmd, 0);
-   return rc;
+   goto err_free_cmd;
}
 
rc = target_xcopy_issue_pt_cmd(xpt_cmd);
if (rc < 0) {
ec_cmd->scsi_status = xpt_cmd->se_cmd.scsi_status;
se_cmd->se_cmd_flags &= ~SCF_PASSTHROUGH_SG_TO_MEM_NOALLOC;
-   transport_generic_free_cmd(se_cmd, 0);
-   return rc;
+   goto err_free_cmd;
}
 
return 0;
+
+err_free_cmd:
+   transport_generic_free_cmd(se_cmd, 0);
+   return rc;
 }
 
 static void target_xcopy_do_work(struct work_struct *work)
@@ -801,10 +803,10 @@ static void target_xcopy_do_work(struct work_struct *work)
unsigned short nolb, cur_nolb, max_nolb, copied_nolb = 0;
 
if (target_parse_xcopy_cmd(xop) != TCM_NO_SENSE)
-   goto err_free;
+   goto err_free_xop;
 
if (WARN_ON_ONCE(!xop->src_dev) || WARN_ON_ONCE(!xop->dst_dev))
-   goto err_free;
+   goto err_free_xop;
 
src_dev = xop->src_dev;
dst_dev = xop->dst_dev;
@@ -835,7 +837,7 @@ static void target_xcopy_do_work(struct work_struct *work)
 
rc = target_xcopy_read_source(ec_cmd, xop, src_dev, src_lba, 
cur_nolb);
if (rc < 0)
-   goto out;
+   goto err_undepend_device;
 
src_lba += cur_nolb;
pr_debug("target_xcopy_do_work: Incremented READ src_lba to 
%llu\n",
@@ -846,10 +848,8 @@ static void target_xcopy_do_work(struct work_struct *work)
 
rc = target_xcopy_write_destination(ec_cmd, xop, dst_dev,
dst_lba, cur_nolb);
-   if (rc < 0) {
-   transport_generic_free_cmd(>src_pt_cmd->se_cmd, 0);
-   goto out;
-   }
+   if (rc < 0)
+   goto err_free_cmd;
 
dst_lba += cur_nolb;
pr_debug("target_xcopy_do_work: Incremented WRITE dst_lba to 
%llu\n",
@@ -876,10 +876,11 @@ static void target_xcopy_do_work(struct work_struct *work)
target_complete_cmd(ec_cmd, SAM_STAT_GOOD);
return;
 
-out:
+err_free_cmd:
+   transport_generic_free_cmd(>src_pt_cmd->se_cmd, 0);
+err_undepend_device:
xcopy_pt_undepend_remotedev(xop);
-
-err_free:
+err_free_xop:
kfree(xop);
/*
 * Don't override an error scsi status if it has already been set
-- 
2.15.0

[PATCH 0/4] target: Adjustments for four function implementations

[SF Markus Elfring]

From: Markus Elfring 
Date: Sat, 4 Nov 2017 14:09:09 +0100

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (4):
  Use common error handling code in three functions
  Combine two condition checks into one statement in target_xcopy_do_work()
  Improve a size determination in three functions
  Delete an error message for a failed memory allocation in two functions

 drivers/target/target_core_xcopy.c | 69 ++
 1 file changed, 33 insertions(+), 36 deletions(-)

-- 
2.15.0

[PATCH] iSCSI-target: Use common error handling code in iscsi_decode_text_input()

[SF Markus Elfring]

From: Markus Elfring 
Date: Fri, 3 Nov 2017 22:20:38 +0100

Add a jump target so that a bit of exception handling can be better reused
at the end of this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/target/iscsi/iscsi_target_parameters.c | 39 --
 1 file changed, 18 insertions(+), 21 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target_parameters.c 
b/drivers/target/iscsi/iscsi_target_parameters.c
index caab1045742d..29a37b242d30 100644
--- a/drivers/target/iscsi/iscsi_target_parameters.c
+++ b/drivers/target/iscsi/iscsi_target_parameters.c
@@ -1380,10 +1380,8 @@ int iscsi_decode_text_input(
char *key, *value;
struct iscsi_param *param;
 
-   if (iscsi_extract_key_value(start, , ) < 0) {
-   kfree(tmpbuf);
-   return -1;
-   }
+   if (iscsi_extract_key_value(start, , ) < 0)
+   goto free_buffer;
 
pr_debug("Got key: %s=%s\n", key, value);
 
@@ -1396,38 +1394,37 @@ int iscsi_decode_text_input(
 
param = iscsi_check_key(key, phase, sender, param_list);
if (!param) {
-   if (iscsi_add_notunderstood_response(key,
-   value, param_list) < 0) {
-   kfree(tmpbuf);
-   return -1;
-   }
+   if (iscsi_add_notunderstood_response(key, value,
+param_list) < 0)
+   goto free_buffer;
+
start += strlen(key) + strlen(value) + 2;
continue;
}
-   if (iscsi_check_value(param, value) < 0) {
-   kfree(tmpbuf);
-   return -1;
-   }
+   if (iscsi_check_value(param, value) < 0)
+   goto free_buffer;
 
start += strlen(key) + strlen(value) + 2;
 
if (IS_PSTATE_PROPOSER(param)) {
-   if (iscsi_check_proposer_state(param, value) < 0) {
-   kfree(tmpbuf);
-   return -1;
-   }
+   if (iscsi_check_proposer_state(param, value) < 0)
+   goto free_buffer;
+
SET_PSTATE_RESPONSE_GOT(param);
} else {
-   if (iscsi_check_acceptor_state(param, value, conn) < 0) 
{
-   kfree(tmpbuf);
-   return -1;
-   }
+   if (iscsi_check_acceptor_state(param, value, conn) < 0)
+   goto free_buffer;
+
SET_PSTATE_ACCEPTOR(param);
}
}
 
kfree(tmpbuf);
return 0;
+
+free_buffer:
+   kfree(tmpbuf);
+   return -1;
 }
 
 int iscsi_encode_text_output(
-- 
2.15.0

[PATCH] SCSI-dpt_i2o: Use common error handling code in adpt_hba_reset()

[SF Markus Elfring]

From: Markus Elfring 
Date: Thu, 2 Nov 2017 17:45:14 +0100

* Adjust five function calls together with a variable assignment.

* Add a jump target so that a bit of exception handling can be better
  reused at the end of this function.

  This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/scsi/dpt_i2o.c | 41 ++---
 1 file changed, 22 insertions(+), 19 deletions(-)

diff --git a/drivers/scsi/dpt_i2o.c b/drivers/scsi/dpt_i2o.c
index fd172b0890d3..cbe1a36fb531 100644
--- a/drivers/scsi/dpt_i2o.c
+++ b/drivers/scsi/dpt_i2o.c
@@ -832,37 +832,40 @@ static int adpt_hba_reset(adpt_hba* pHba)
pHba->state |= DPTI_STATE_RESET;
 
// Activate does get status , init outbound, and get hrt
-   if ((rcode=adpt_i2o_activate_hba(pHba)) < 0) {
+   rcode = adpt_i2o_activate_hba(pHba);
+   if (rcode < 0) {
printk(KERN_ERR "%s: Could not activate\n", pHba->name);
-   adpt_i2o_delete_hba(pHba);
-   return rcode;
+   goto delete_hba;
}
 
-   if ((rcode=adpt_i2o_build_sys_table()) < 0) {
-   adpt_i2o_delete_hba(pHba);
-   return rcode;
-   }
+   rcode = adpt_i2o_build_sys_table();
+   if (rcode < 0)
+   goto delete_hba;
+
PDEBUG("%s: in HOLD state\n",pHba->name);
 
-   if ((rcode=adpt_i2o_online_hba(pHba)) < 0) {
-   adpt_i2o_delete_hba(pHba);  
-   return rcode;
-   }
+   rcode = adpt_i2o_online_hba(pHba);
+   if (rcode < 0)
+   goto delete_hba;
+
PDEBUG("%s: in OPERATIONAL state\n",pHba->name);
 
-   if ((rcode=adpt_i2o_lct_get(pHba)) < 0){
-   adpt_i2o_delete_hba(pHba);
-   return rcode;
-   }
+   rcode = adpt_i2o_lct_get(pHba);
+   if (rcode < 0)
+   goto delete_hba;
+
+   rcode = adpt_i2o_reparse_lct(pHba);
+   if (rcode < 0)
+   goto delete_hba;
 
-   if ((rcode=adpt_i2o_reparse_lct(pHba)) < 0){
-   adpt_i2o_delete_hba(pHba);
-   return rcode;
-   }
pHba->state &= ~DPTI_STATE_RESET;
 
adpt_fail_posted_scbs(pHba);
return 0;   /* return success */
+
+delete_hba:
+   adpt_i2o_delete_hba(pHba);
+   return rcode;
 }
 
 /*===
-- 
2.15.0

Checking the description for aac_comm_init()

[SF Markus Elfring]

Hello,

I have taken another look also at the implementation of the
function “aac_comm_init”.

http://elixir.free-electrons.com/linux/v4.14-rc7/source/drivers/scsi/aacraid/comminit.c#L333
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/scsi/aacraid/comminit.c?id=fa8785e862ef644f742558f1a8c91eca6f3f0004#n345

Can it be that the description for the possible return values is questionable
in the corresponding comment block at the moment?

Regards,
Markus

[PATCH 3/3] SCSI-sg: Fix a typo in a comment line in sg_ioctl()

[SF Markus Elfring]

From: Markus Elfring 
Date: Fri, 25 Aug 2017 22:06:59 +0200

Fix a word in this description.

Signed-off-by: Markus Elfring 
---
 drivers/scsi/sg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 4a2db7ff14cc..c80899d249c6 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -891,7 +891,7 @@ sg_ioctl(struct file *filp, unsigned int cmd_in, unsigned 
long arg)
case SG_SET_FORCE_LOW_DMA:
/*
 * N.B. This ioctl never worked properly, but failed to
-* return an error value. So returning '0' to keep compability
+* return an error value. So returning '0' to keep compatibility
 * with legacy applications.
 */
return 0;
-- 
2.14.0

[PATCH 2/3] SCSI-sg: Improve a size determination in sg_alloc()

[SF Markus Elfring]

From: Markus Elfring 
Date: Fri, 25 Aug 2017 21:55:14 +0200

Replace the specification of a data type by a pointer dereference
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.

Signed-off-by: Markus Elfring 
---
 drivers/scsi/sg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 8ca10a2fc1a0..4a2db7ff14cc 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -1403,7 +1403,7 @@ sg_alloc(struct gendisk *disk, struct scsi_device *scsidp)
int error;
u32 k;
 
-   sdp = kzalloc(sizeof(Sg_device), GFP_KERNEL);
+   sdp = kzalloc(sizeof(*sdp), GFP_KERNEL);
if (!sdp)
return ERR_PTR(-ENOMEM);
 
-- 
2.14.0

[PATCH 1/3] SCSI-sg: Delete an error message for a failed memory allocation in sg_alloc()

[SF Markus Elfring]

From: Markus Elfring 
Date: Fri, 25 Aug 2017 21:48:11 +0200

Omit an extra message for a memory allocation failure in this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/scsi/sg.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index d7ff71e0c85c..8ca10a2fc1a0 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -1404,11 +1404,8 @@ sg_alloc(struct gendisk *disk, struct scsi_device 
*scsidp)
u32 k;
 
sdp = kzalloc(sizeof(Sg_device), GFP_KERNEL);
-   if (!sdp) {
-   sdev_printk(KERN_WARNING, scsidp, "%s: kmalloc Sg_device "
-   "failure\n", __func__);
+   if (!sdp)
return ERR_PTR(-ENOMEM);
-   }
 
idr_preload(GFP_KERNEL);
write_lock_irqsave(_index_lock, iflags);
-- 
2.14.0

[PATCH 0/3] SCSI-sg: Adjustments for two function implementations

[SF Markus Elfring]

From: Markus Elfring 
Date: Fri, 25 Aug 2017 22:20:02 +0200

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (3):
  Delete an error message for a failed memory allocation in sg_alloc()
  Improve a size determination in sg_alloc()
  Fix a typo in a comment line in sg_ioctl()

 drivers/scsi/sg.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

-- 
2.14.0

[PATCH] SCSI-sd: Delete an error message for a failed memory allocation in sd_revalidate_disk()

[SF Markus Elfring]

From: Markus Elfring 
Date: Fri, 25 Aug 2017 21:34:44 +0200

Omit an extra message for a memory allocation failure in this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/scsi/sd.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index bea36adeee17..1f15c31e5a8a 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -3067,11 +3067,8 @@ static int sd_revalidate_disk(struct gendisk *disk)
goto out;
 
buffer = kmalloc(SD_BUF_SIZE, GFP_KERNEL);
-   if (!buffer) {
-   sd_printk(KERN_WARNING, sdkp, "sd_revalidate_disk: Memory "
- "allocation failure.\n");
+   if (!buffer)
goto out;
-   }
 
sd_spinup_disk(sdkp);
 
-- 
2.14.0

[PATCH] SCSI-scan: Delete an error message for a failed memory allocation in two functions

[SF Markus Elfring]

From: Markus Elfring 
Date: Fri, 25 Aug 2017 21:17:02 +0200

Omit an extra message for a memory allocation failure in these functions.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/scsi/scsi_scan.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index fd88dabd599d..280ac20a644b 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -427,10 +427,9 @@ static struct scsi_target *scsi_alloc_target(struct device 
*parent,
int error, ref_got;
 
starget = kzalloc(size, GFP_KERNEL);
-   if (!starget) {
-   printk(KERN_ERR "%s: allocation failure\n", __func__);
+   if (!starget)
return NULL;
-   }
+
dev = >dev;
device_initialize(dev);
kref_init(>reap_ref);
@@ -1340,10 +1339,8 @@ static int scsi_report_lun_scan(struct scsi_target 
*starget, int bflags,
 retry:
lun_data = kmalloc(length, GFP_KERNEL |
   (sdev->host->unchecked_isa_dma ? __GFP_DMA : 0));
-   if (!lun_data) {
-   printk(ALLOC_FAILURE_MSG, __func__);
+   if (!lun_data)
goto out;
-   }
 
scsi_cmd[0] = REPORT_LUNS;
 
-- 
2.14.0

[PATCH] SCSI-devinfo: Delete an error message for a failed memory allocation in scsi_dev_info_list_add_keyed()

[SF Markus Elfring]

From: Markus Elfring 
Date: Fri, 25 Aug 2017 20:54:16 +0200

Omit an extra message for a memory allocation failure in this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/scsi/scsi_devinfo.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c
index 28fea83ae2fe..3009934b95a7 100644
--- a/drivers/scsi/scsi_devinfo.c
+++ b/drivers/scsi/scsi_devinfo.c
@@ -369,10 +369,8 @@ int scsi_dev_info_list_add_keyed(int compatible, char 
*vendor, char *model,
return PTR_ERR(devinfo_table);
 
devinfo = kmalloc(sizeof(*devinfo), GFP_KERNEL);
-   if (!devinfo) {
-   printk(KERN_ERR "%s: no memory\n", __func__);
+   if (!devinfo)
return -ENOMEM;
-   }
 
scsi_strcpy_devinfo("vendor", devinfo->vendor, sizeof(devinfo->vendor),
vendor, compatible);
-- 
2.14.0

Re: [PATCH 0/5] block-cciss: Fine-tuning for two function implementations

[SF Markus Elfring]

> Date: Thu, 18 Aug 2016 11:40:04 +0200
> 
> Some update suggestions were taken into account
> from static source code analysis.
> 
> Markus Elfring (5):
>   Use memdup_user()
>   Less function calls after error detection
>   Delete unnecessary initialisations
>   Move an assignment for the variable "sg_used"
>   Replace three kzalloc() calls by kcalloc()
> 
>  drivers/block/cciss.c | 66 
> ---
>  1 file changed, 31 insertions(+), 35 deletions(-)

How will the clarification be continued for the shown change possibilities?

Regards,
Markus

Re: Checking error messages for failed memory allocations

[SF Markus Elfring]

> Feel free to submit documentation patches.

Do involved software developers agree on the functionality for
stack dumps because of out of memory situations?

Regards,
Markus

Re: Checking error messages for failed memory allocations

[SF Markus Elfring]

> Basically most everything that has a gfp_t argument does a
> dump_stack() on OOM unless __GFP_NOWARN is specified by that gfp_t.

How do you think about to improve any programming interface documentation
around such a function property?

Are there any special checks needed for function implementations
which can pass the flag “__GFP_NOWARN”?

Regards,
Markus

Re: scsi: ufs: Delete an error message for a failed memory allocation in ufshcd_memory_alloc()

[SF Markus Elfring]

> Although i don't know "out of memory" messages will be printed out by 
> dmam_alloc_coherent() APIs
> or not.

Would such information belong to the programming interface documentation?

Are there any related tags or source code annotations needed?

Regards,
Markus

[PATCH 3/3] scsi: ufs: Delete an unnecessary return statement in ufshcd_exception_event_handler()

[SF Markus Elfring]

From: Markus Elfring 
Date: Tue, 25 Apr 2017 22:00:05 +0200

The script "checkpatch.pl" pointed information out like the following.

WARNING: void function return statements are not generally useful

Thus remove such a statement here.

Signed-off-by: Markus Elfring 
---
 drivers/scsi/ufs/ufshcd.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 5216e33e61a3..9018f26a5667 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -4966,7 +4966,6 @@ static void ufshcd_exception_event_handler(struct 
work_struct *work)
 
 out:
pm_runtime_put_sync(hba->dev);
-   return;
 }
 
 /* Complete requests that have door-bell cleared */
-- 
2.12.2

[PATCH 2/3] scsi: ufs: Delete an error message for a failed memory allocation in ufshcd_memory_alloc()

[SF Markus Elfring]

From: Markus Elfring 
Date: Tue, 25 Apr 2017 21:50:43 +0200

The script "checkpatch.pl" pointed information out like the following.

WARNING: Possible unnecessary 'out of memory' message

Thus remove such a statement here.

Link: 
http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
Signed-off-by: Markus Elfring 
---
 drivers/scsi/ufs/ufshcd.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index ce385911a20e..5216e33e61a3 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -3274,8 +3274,7 @@ static int ufshcd_memory_alloc(struct ufs_hba *hba)
GFP_KERNEL);
-   if (!hba->lrb) {
-   dev_err(hba->dev, "LRB Memory allocation failed\n");
+   if (!hba->lrb)
goto out;
-   }
+
return 0;
 out:
return -ENOMEM;
-- 
2.12.2

[PATCH 1/3] scsi: ufs: Use devm_kcalloc() in ufshcd_memory_alloc()

[SF Markus Elfring]

From: Markus Elfring 
Date: Tue, 25 Apr 2017 21:45:25 +0200

* A multiplication for the size determination of a memory allocation
  indicated that an array data structure should be processed.
  Thus use the corresponding function "devm_kcalloc".

* Replace the specification of a data structure by a pointer dereference
  to make the corresponding size determination a bit safer according to
  the Linux coding style convention.

Signed-off-by: Markus Elfring 
---
 drivers/scsi/ufs/ufshcd.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 9ef8ce7f01a2..ce385911a20e 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -3270,8 +3270,7 @@ static int ufshcd_memory_alloc(struct ufs_hba *hba)
}
 
/* Allocate memory for local reference block */
-   hba->lrb = devm_kzalloc(hba->dev,
-   hba->nutrs * sizeof(struct ufshcd_lrb),
+   hba->lrb = devm_kcalloc(hba->dev, hba->nutrs, sizeof(*hba->lrb),
GFP_KERNEL);
if (!hba->lrb) {
dev_err(hba->dev, "LRB Memory allocation failed\n");
-- 
2.12.2

[PATCH 0/3] SCSI-UFSHCD: Fine-tuning for two function implementations

[SF Markus Elfring]

From: Markus Elfring 
Date: Tue, 25 Apr 2017 22:20:02 +0200

Three update suggestions were taken into account
from static source code analysis.

Markus Elfring (3):
  Use devm_kcalloc() in ufshcd_memory_alloc()
  Delete an error message for a failed memory allocation in 
ufshcd_memory_alloc()
  Delete an unnecessary return statement in ufshcd_exception_event_handler()

 drivers/scsi/ufs/ufshcd.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

-- 
2.12.2

[PATCH 4/5] target: Use kmalloc_array() in compare_and_write_callback()

[SF Markus Elfring]

From: Markus Elfring 
Date: Sun, 9 Apr 2017 20:25:11 +0200

* A multiplication for the size determination of a memory allocation
  indicated that an array data structure should be processed.
  Thus use the corresponding function "kmalloc_array".

  This issue was detected by using the Coccinelle software.

* Replace the specification of a data structure by a pointer dereference
  to make the corresponding size determination a bit safer according to
  the Linux coding style convention.

Signed-off-by: Markus Elfring 
---
 drivers/target/target_core_sbc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c
index ee35c90e3b8d..a7fa4a7339db 100644
--- a/drivers/target/target_core_sbc.c
+++ b/drivers/target/target_core_sbc.c
@@ -519,8 +519,8 @@ static sense_reason_t compare_and_write_callback(struct 
se_cmd *cmd, bool succes
goto out;
}
 
-   write_sg = kmalloc(sizeof(struct scatterlist) * cmd->t_data_nents,
-  GFP_KERNEL);
+   write_sg = kmalloc_array(cmd->t_data_nents, sizeof(*write_sg),
+GFP_KERNEL);
if (!write_sg) {
pr_err("Unable to allocate compare_and_write sg\n");
ret = TCM_OUT_OF_RESOURCES;
-- 
2.12.2

[PATCH 3/5] target: Improve size determinations in two functions

[SF Markus Elfring]

From: Markus Elfring 
Date: Sun, 9 Apr 2017 20:15:12 +0200

Replace the specification of two data structures by pointer dereferences
as the parameter for the operator "sizeof" to make the corresponding size
determinations a bit safer according to the Linux coding style convention.

Signed-off-by: Markus Elfring 
---
 drivers/target/target_core_rd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/target/target_core_rd.c b/drivers/target/target_core_rd.c
index 838dc128d494..20253d04103f 100644
--- a/drivers/target/target_core_rd.c
+++ b/drivers/target/target_core_rd.c
@@ -47,7 +47,7 @@ static int rd_attach_hba(struct se_hba *hba, u32 host_id)
 {
struct rd_host *rd_host;
 
-   rd_host = kzalloc(sizeof(struct rd_host), GFP_KERNEL);
+   rd_host = kzalloc(sizeof(*rd_host), GFP_KERNEL);
if (!rd_host)
return -ENOMEM;
 
@@ -285,7 +285,7 @@ static struct se_device *rd_alloc_device(struct se_hba 
*hba, const char *name)
struct rd_dev *rd_dev;
struct rd_host *rd_host = hba->hba_ptr;
 
-   rd_dev = kzalloc(sizeof(struct rd_dev), GFP_KERNEL);
+   rd_dev = kzalloc(sizeof(*rd_dev), GFP_KERNEL);
if (!rd_dev)
return NULL;
 
-- 
2.12.2

[PATCH 2/5] target: Delete error messages for failed memory allocations

[SF Markus Elfring]

From: Markus Elfring 
Date: Sun, 9 Apr 2017 19:20:26 +0200

The script "checkpatch.pl" pointed information out like the following.

WARNING: Possible unnecessary 'out of memory' message

Thus remove such statements here.

Link: 
http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
Signed-off-by: Markus Elfring 
---
 drivers/target/target_core_rd.c | 23 +--
 1 file changed, 5 insertions(+), 18 deletions(-)

diff --git a/drivers/target/target_core_rd.c b/drivers/target/target_core_rd.c
index 026857861107..838dc128d494 100644
--- a/drivers/target/target_core_rd.c
+++ b/drivers/target/target_core_rd.c
@@ -48,10 +48,8 @@ static int rd_attach_hba(struct se_hba *hba, u32 host_id)
struct rd_host *rd_host;
 
rd_host = kzalloc(sizeof(struct rd_host), GFP_KERNEL);
-   if (!rd_host) {
-   pr_err("Unable to allocate memory for struct rd_host\n");
+   if (!rd_host)
return -ENOMEM;
-   }
 
rd_host->rd_host_id = host_id;
 
@@ -148,11 +146,8 @@ static int rd_allocate_sgl_table(struct rd_dev *rd_dev, 
struct rd_dev_sg_table *
 
sg = kcalloc(sg_per_table + chain_entry, sizeof(*sg),
GFP_KERNEL);
-   if (!sg) {
-   pr_err("Unable to allocate scatterlist array"
-   " for struct rd_dev\n");
+   if (!sg)
return -ENOMEM;
-   }
 
sg_init_table(sg, sg_per_table + chain_entry);
 
@@ -211,11 +206,8 @@ static int rd_build_device_space(struct rd_dev *rd_dev)
 
sg_tables = (total_sg_needed / max_sg_per_table) + 1;
sg_table = kcalloc(sg_tables, sizeof(*sg_table), GFP_KERNEL);
-   if (!sg_table) {
-   pr_err("Unable to allocate memory for Ramdisk"
-  " scatterlist tables\n");
+   if (!sg_table)
return -ENOMEM;
-   }
 
rd_dev->sg_table_array = sg_table;
rd_dev->sg_table_count = sg_tables;
@@ -271,11 +263,8 @@ static int rd_build_prot_space(struct rd_dev *rd_dev, int 
prot_length, int block
 
sg_tables = (total_sg_needed / max_sg_per_table) + 1;
sg_table = kcalloc(sg_tables, sizeof(*sg_table), GFP_KERNEL);
-   if (!sg_table) {
-   pr_err("Unable to allocate memory for Ramdisk protection"
-  " scatterlist tables\n");
+   if (!sg_table)
return -ENOMEM;
-   }
 
rd_dev->sg_prot_array = sg_table;
rd_dev->sg_prot_count = sg_tables;
@@ -297,10 +286,8 @@ static struct se_device *rd_alloc_device(struct se_hba 
*hba, const char *name)
struct rd_host *rd_host = hba->hba_ptr;
 
rd_dev = kzalloc(sizeof(struct rd_dev), GFP_KERNEL);
-   if (!rd_dev) {
-   pr_err("Unable to allocate memory for struct rd_dev\n");
+   if (!rd_dev)
return NULL;
-   }
 
rd_dev->rd_host = rd_host;
 
-- 
2.12.2

[PATCH 1/5] target: Use kcalloc() in two functions

[SF Markus Elfring]

From: Markus Elfring 
Date: Sun, 9 Apr 2017 19:02:38 +0200

* Multiplications for the size determination of memory allocations
  indicated that array data structures should be processed.
  Thus use the corresponding function "kcalloc".

  This issue was detected by using the Coccinelle software.

* Replace the specification of data structures by pointer dereferences
  to make the corresponding size determination a bit safer according to
  the Linux coding style convention.

Signed-off-by: Markus Elfring 
---
 drivers/target/target_core_rd.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/target/target_core_rd.c b/drivers/target/target_core_rd.c
index 5f23f341f8d3..026857861107 100644
--- a/drivers/target/target_core_rd.c
+++ b/drivers/target/target_core_rd.c
@@ -210,8 +210,7 @@ static int rd_build_device_space(struct rd_dev *rd_dev)
total_sg_needed = rd_dev->rd_page_count;
 
sg_tables = (total_sg_needed / max_sg_per_table) + 1;
-
-   sg_table = kzalloc(sg_tables * sizeof(struct rd_dev_sg_table), 
GFP_KERNEL);
+   sg_table = kcalloc(sg_tables, sizeof(*sg_table), GFP_KERNEL);
if (!sg_table) {
pr_err("Unable to allocate memory for Ramdisk"
   " scatterlist tables\n");
@@ -271,8 +270,7 @@ static int rd_build_prot_space(struct rd_dev *rd_dev, int 
prot_length, int block
total_sg_needed = (rd_dev->rd_page_count * prot_length / block_size) + 
1;
 
sg_tables = (total_sg_needed / max_sg_per_table) + 1;
-
-   sg_table = kzalloc(sg_tables * sizeof(struct rd_dev_sg_table), 
GFP_KERNEL);
+   sg_table = kcalloc(sg_tables, sizeof(*sg_table), GFP_KERNEL);
if (!sg_table) {
pr_err("Unable to allocate memory for Ramdisk protection"
   " scatterlist tables\n");
-- 
2.12.2

[PATCH 0/5] target: Fine-tuning for some function implementations

[SF Markus Elfring]

From: Markus Elfring 
Date: Sun, 9 Apr 2017 21:33:21 +0200

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (5):
  rd: Use kcalloc() in two functions
  rd: Delete error messages for failed memory allocations
  rd: Improve size determinations in two functions
  sbc: Use kmalloc_array() in compare_and_write_callback()
  transport: Use kmalloc_array() in transport_kmap_data_sg()

 drivers/target/target_core_rd.c| 33 +
 drivers/target/target_core_sbc.c   |  4 ++--
 drivers/target/target_core_transport.c |  2 +-
 3 files changed, 12 insertions(+), 27 deletions(-)

-- 
2.12.2

[PATCH 3/3] iscsi-target: Improve size determinations in four functions

[SF Markus Elfring]

From: Markus Elfring 
Date: Sun, 9 Apr 2017 16:00:39 +0200

Replace the specification of four data structures by pointer dereferences
as the parameter for the operator "sizeof" to make the corresponding size
determinations a bit safer according to the Linux coding style convention.

Signed-off-by: Markus Elfring 
---
 drivers/target/iscsi/iscsi_target.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target.c 
b/drivers/target/iscsi/iscsi_target.c
index 196e9dd138ca..5ef6eb238174 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -128,7 +128,7 @@ struct iscsi_tiqn *iscsit_add_tiqn(unsigned char *buf)
return ERR_PTR(-EINVAL);
}
 
-   tiqn = kzalloc(sizeof(struct iscsi_tiqn), GFP_KERNEL);
+   tiqn = kzalloc(sizeof(*tiqn), GFP_KERNEL);
if (!tiqn)
return ERR_PTR(-ENOMEM);
 
@@ -360,7 +360,7 @@ struct iscsi_np *iscsit_add_np(
return np;
}
 
-   np = kzalloc(sizeof(struct iscsi_np), GFP_KERNEL);
+   np = kzalloc(sizeof(*np), GFP_KERNEL);
if (!np) {
mutex_unlock(_lock);
return ERR_PTR(-ENOMEM);
@@ -694,8 +694,7 @@ static int __init iscsi_target_init_module(void)
int ret = 0, size;
 
pr_debug("iSCSI-Target "ISCSIT_VERSION"\n");
-
-   iscsit_global = kzalloc(sizeof(struct iscsit_global), GFP_KERNEL);
+   iscsit_global = kzalloc(sizeof(*iscsit_global), GFP_KERNEL);
if (!iscsit_global)
return -1;
 
@@ -1987,8 +1986,7 @@ iscsit_handle_task_mgt_cmd(struct iscsi_conn *conn, 
struct iscsi_cmd *cmd,
hdr->refcmdsn = cpu_to_be32(ISCSI_RESERVED_TAG);
 
cmd->data_direction = DMA_NONE;
-
-   cmd->tmr_req = kzalloc(sizeof(struct iscsi_tmr_req), GFP_KERNEL);
+   cmd->tmr_req = kzalloc(sizeof(*cmd->tmr_req), GFP_KERNEL);
if (!cmd->tmr_req)
return iscsit_add_reject_cmd(cmd,
 ISCSI_REASON_BOOKMARK_NO_RESOURCES,
-- 
2.12.2

[PATCH 2/3] iscsi-target: Delete error messages for failed memory allocations

[SF Markus Elfring]

From: Markus Elfring 
Date: Sun, 9 Apr 2017 15:34:50 +0200

The script "checkpatch.pl" pointed information out like the following.

WARNING: Possible unnecessary 'out of memory' message

Thus remove such statements here.

Link: 
http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
Signed-off-by: Markus Elfring 
---
 drivers/target/iscsi/iscsi_target.c | 37 ++---
 1 file changed, 10 insertions(+), 27 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target.c 
b/drivers/target/iscsi/iscsi_target.c
index 536b939afe9d..196e9dd138ca 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -129,10 +129,8 @@ struct iscsi_tiqn *iscsit_add_tiqn(unsigned char *buf)
}
 
tiqn = kzalloc(sizeof(struct iscsi_tiqn), GFP_KERNEL);
-   if (!tiqn) {
-   pr_err("Unable to allocate struct iscsi_tiqn\n");
+   if (!tiqn)
return ERR_PTR(-ENOMEM);
-   }
 
sprintf(tiqn->tiqn, "%s", buf);
INIT_LIST_HEAD(>tiqn_list);
@@ -364,7 +362,6 @@ struct iscsi_np *iscsit_add_np(
 
np = kzalloc(sizeof(struct iscsi_np), GFP_KERNEL);
if (!np) {
-   pr_err("Unable to allocate memory for struct iscsi_np\n");
mutex_unlock(_lock);
return ERR_PTR(-ENOMEM);
}
@@ -699,10 +696,9 @@ static int __init iscsi_target_init_module(void)
pr_debug("iSCSI-Target "ISCSIT_VERSION"\n");
 
iscsit_global = kzalloc(sizeof(struct iscsit_global), GFP_KERNEL);
-   if (!iscsit_global) {
-   pr_err("Unable to allocate memory for iscsit_global\n");
+   if (!iscsit_global)
return -1;
-   }
+
spin_lock_init(_global->ts_bitmap_lock);
mutex_init(_id_lock);
spin_lock_init(_idr_lock);
@@ -715,10 +711,8 @@ static int __init iscsi_target_init_module(void)
 
size = BITS_TO_LONGS(ISCSIT_BITMAP_BITS) * sizeof(long);
iscsit_global->ts_bitmap = vzalloc(size);
-   if (!iscsit_global->ts_bitmap) {
-   pr_err("Unable to allocate iscsit_global->ts_bitmap\n");
+   if (!iscsit_global->ts_bitmap)
goto configfs_out;
-   }
 
lio_qr_cache = kmem_cache_create("lio_qr_cache",
sizeof(struct iscsi_queue_req),
@@ -986,10 +980,8 @@ static int iscsit_allocate_iovecs(struct iscsi_cmd *cmd)
 
iov_count += ISCSI_IOV_DATA_BUFFER;
cmd->iov_data = kcalloc(iov_count, sizeof(*cmd->iov_data), GFP_KERNEL);
-   if (!cmd->iov_data) {
-   pr_err("Unable to allocate cmd->iov_data\n");
+   if (!cmd->iov_data)
return -ENOMEM;
-   }
 
cmd->orig_iov_data_count = iov_count;
return 0;
@@ -1850,8 +1842,6 @@ static int iscsit_handle_nop_out(struct iscsi_conn *conn, 
struct iscsi_cmd *cmd,
 
ping_data = kzalloc(payload_length + 1, GFP_KERNEL);
if (!ping_data) {
-   pr_err("Unable to allocate memory for"
-   " NOPOUT ping data.\n");
ret = -1;
goto out;
}
@@ -1999,13 +1989,10 @@ iscsit_handle_task_mgt_cmd(struct iscsi_conn *conn, 
struct iscsi_cmd *cmd,
cmd->data_direction = DMA_NONE;
 
cmd->tmr_req = kzalloc(sizeof(struct iscsi_tmr_req), GFP_KERNEL);
-   if (!cmd->tmr_req) {
-   pr_err("Unable to allocate memory for"
-   " Task Management command!\n");
+   if (!cmd->tmr_req)
return iscsit_add_reject_cmd(cmd,
 ISCSI_REASON_BOOKMARK_NO_RESOURCES,
 buf);
-   }
 
/*
 * TASK_REASSIGN for ERL=2 / connection stays inside of
@@ -2265,11 +2252,9 @@ iscsit_handle_text_cmd(struct iscsi_conn *conn, struct 
iscsi_cmd *cmd,
struct kvec iov[3];
 
text_in = kzalloc(payload_length, GFP_KERNEL);
-   if (!text_in) {
-   pr_err("Unable to allocate memory for"
-   " incoming text parameters\n");
+   if (!text_in)
goto reject;
-   }
+
cmd->text_in_ptr = text_in;
 
memset(iov, 0, 3 * sizeof(struct kvec));
@@ -3353,11 +3338,9 @@ iscsit_build_sendtargets_response(struct iscsi_cmd *cmd,
 SENDTARGETS_BUF_LIMIT);
 
payload = kzalloc(buffer_len, GFP_KERNEL);
-   if (!payload) {
-   pr_err("Unable to allocate memory for sendtargets"
-   " response.\n");
+   if (!payload)
return -ENOMEM;
-   }
+
/*
 * Locate pointer to iqn./eui. string for ICF_SENDTARGETS_SINGLE
 * explicit case..
-- 
2.12.2

[PATCH 1/3] iscsi-target: Use kcalloc() in iscsit_allocate_iovecs()

[SF Markus Elfring]

From: Markus Elfring 
Date: Sun, 9 Apr 2017 15:06:00 +0200

* A multiplication for the size determination of a memory allocation
  indicated that an array data structure should be processed.
  Thus use the corresponding function "kcalloc".

  This issue was detected by using the Coccinelle software.

* Replace the specification of a data structure by a pointer dereference
  to make the corresponding size determination a bit safer according to
  the Linux coding style convention.

Signed-off-by: Markus Elfring 
---
 drivers/target/iscsi/iscsi_target.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target.c 
b/drivers/target/iscsi/iscsi_target.c
index a91802432f2f..536b939afe9d 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -985,8 +985,7 @@ static int iscsit_allocate_iovecs(struct iscsi_cmd *cmd)
u32 iov_count = max(1UL, DIV_ROUND_UP(cmd->se_cmd.data_length, 
PAGE_SIZE));
 
iov_count += ISCSI_IOV_DATA_BUFFER;
-
-   cmd->iov_data = kzalloc(iov_count * sizeof(struct kvec), GFP_KERNEL);
+   cmd->iov_data = kcalloc(iov_count, sizeof(*cmd->iov_data), GFP_KERNEL);
if (!cmd->iov_data) {
pr_err("Unable to allocate cmd->iov_data\n");
return -ENOMEM;
-- 
2.12.2

[PATCH 0/3] iSCSI-target: Fine-tuning for some function implementations

[SF Markus Elfring]

From: Markus Elfring 
Date: Sun, 9 Apr 2017 16:11:23 +0200

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (3):
  Use kcalloc() in iscsit_allocate_iovecs()
  Delete error messages for failed memory allocations
  Improve size determinations in four functions

 drivers/target/iscsi/iscsi_target.c | 50 +++--
 1 file changed, 15 insertions(+), 35 deletions(-)

-- 
2.12.2

Re: [PATCH 1/7] aacraid: Use memdup_user() rather than duplicating its implementation

[SF Markus Elfring]

>> @@ -526,15 +526,9 @@ static int aac_send_raw_srb(struct aac_dev* dev, void
>> __user * arg)
>> goto cleanup;
>> }
>>
>> -   user_srbcmd = kmalloc(fibsize, GFP_KERNEL);
>> -   if (!user_srbcmd) {
>> -   dprintk((KERN_DEBUG"aacraid: Could not make a copy of the 
>> srb\n"));
>> -   rcode = -ENOMEM;
>> -   goto cleanup;
>> -   }
>> -   if(copy_from_user(user_srbcmd, user_srb,fibsize)){
>> -   dprintk((KERN_DEBUG"aacraid: Could not copy srb from 
>> user\n"));
>> -   rcode = -EFAULT;
>> +   user_srbcmd = memdup_user(user_srb, fibsize);
>> +   if (IS_ERR(user_srbcmd)) {
>> +   rcode = PTR_ERR(user_srbcmd);
>> goto cleanup;
>> }
>>
>> --
> 
> Hi Markus,
> 
> Patch 2/7 should precede Patch 1/7, as falling into kfree() would not look 
> pretty.

Do you eventually prefer that this source code adjustment should be combined 
with
the update suggestion "[2/7] aacraid: One function call less in 
aac_send_raw_srb()
after error detection" in a single update step?

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

[PATCH] megaraid_sas: Use memdup_user() rather than duplicating its implementation

[SF Markus Elfring]

From: Markus Elfring 
Date: Sun, 21 Aug 2016 10:39:04 +0200

Reuse existing functionality from memdup_user() instead of keeping
duplicate source code.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/scsi/megaraid/megaraid_sas_base.c | 11 +++
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c 
b/drivers/scsi/megaraid/megaraid_sas_base.c
index c1ed25a..9a2fe4e 100644
--- a/drivers/scsi/megaraid/megaraid_sas_base.c
+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -6711,14 +6711,9 @@ static int megasas_mgmt_ioctl_fw(struct file *file, 
unsigned long arg)
unsigned long flags;
u32 wait_time = MEGASAS_RESET_WAIT_TIME;
 
-   ioc = kmalloc(sizeof(*ioc), GFP_KERNEL);
-   if (!ioc)
-   return -ENOMEM;
-
-   if (copy_from_user(ioc, user_ioc, sizeof(*ioc))) {
-   error = -EFAULT;
-   goto out_kfree_ioc;
-   }
+   ioc = memdup_user(user_ioc, sizeof(*ioc));
+   if (IS_ERR(ioc))
+   return PTR_ERR(ioc);
 
instance = megasas_lookup_instance(ioc->host_no);
if (!instance) {
-- 
2.9.3

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

[PATCH 7/7] aacraid: Apply another recommendation from "checkpatch.pl"

[SF Markus Elfring]

From: Markus Elfring 
Date: Sun, 21 Aug 2016 08:23:25 +0200

The script "checkpatch.pl" can point out that assignments should usually
not be performed within condition checks.
Thus move the assignment for the variable "srbfib" to a separate statement.

Signed-off-by: Markus Elfring 
---
 drivers/scsi/aacraid/commctrl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/aacraid/commctrl.c b/drivers/scsi/aacraid/commctrl.c
index d2029db..7e6c76d 100644
--- a/drivers/scsi/aacraid/commctrl.c
+++ b/drivers/scsi/aacraid/commctrl.c
@@ -499,9 +499,9 @@ static int aac_send_raw_srb(struct aac_dev* dev, void 
__user * arg)
/*
 *  Allocate and initialize a Fib then setup a SRB command
 */
-   if (!(srbfib = aac_fib_alloc(dev))) {
+   srbfib = aac_fib_alloc(dev);
+   if (!srbfib)
return -ENOMEM;
-   }
aac_fib_init(srbfib);
/* raw_srb FIB is not FastResponseCapable */
srbfib->hw_fib_va->header.XferState &= 
~cpu_to_le32(FastResponseCapable);
-- 
2.9.3

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

[PATCH 6/7] aacraid: Improve determination of a few sizes

[SF Markus Elfring]

From: Markus Elfring 
Date: Sun, 21 Aug 2016 08:04:48 +0200

Replace the specification of data structures by references for variables
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.

Signed-off-by: Markus Elfring 
---
 drivers/scsi/aacraid/commctrl.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/aacraid/commctrl.c b/drivers/scsi/aacraid/commctrl.c
index cda03f0..d2029db 100644
--- a/drivers/scsi/aacraid/commctrl.c
+++ b/drivers/scsi/aacraid/commctrl.c
@@ -177,7 +177,7 @@ static int open_getadapter_fib(struct aac_dev * dev, void 
__user *arg)
struct aac_fib_context * fibctx;
int status;
 
-   fibctx = kmalloc(sizeof(struct aac_fib_context), GFP_KERNEL);
+   fibctx = kmalloc(sizeof(*fibctx), GFP_KERNEL);
if (fibctx == NULL) {
status = -ENOMEM;
} else {
@@ -186,7 +186,7 @@ static int open_getadapter_fib(struct aac_dev * dev, void 
__user *arg)
struct aac_fib_context * context;
 
fibctx->type = FSAFS_NTC_GET_ADAPTER_FIB_CONTEXT;
-   fibctx->size = sizeof(struct aac_fib_context);
+   fibctx->size = sizeof(*fibctx);
/*
 *  Yes yes, I know this could be an index, but we have a
 * better guarantee of uniqueness for the locked loop below.
@@ -251,7 +251,7 @@ static int next_getadapter_fib(struct aac_dev * dev, void 
__user *arg)
struct list_head * entry;
unsigned long flags;
 
-   if (copy_from_user((void *), arg, sizeof(struct fib_ioctl)))
+   if (copy_from_user(, arg, sizeof(f)))
return -EFAULT;
/*
 *  Verify that the HANDLE passed in was a valid AdapterFibContext
@@ -509,7 +509,7 @@ static int aac_send_raw_srb(struct aac_dev* dev, void 
__user * arg)
srbcmd = (struct aac_srb*) fib_data(srbfib);
 
memset(sg_list, 0, sizeof(sg_list)); /* cleanup may take issue */
-   if (copy_from_user(, _srb->count, sizeof(u32))) {
+   if (copy_from_user(, _srb->count, sizeof(fibsize))) {
dprintk((KERN_DEBUG"aacraid: Could not copy data size from 
user\n"));
rcode = -EFAULT;
goto free_sg_list;
-- 
2.9.3

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

[PATCH 5/7] aacraid: Add spaces after control flow keywords

[SF Markus Elfring]

From: Markus Elfring 
Date: Sun, 21 Aug 2016 07:10:43 +0200

Keywords which belong to the category "control flow" in the C programming
language should be followed by a space character according to
the Linux coding style convention.

Signed-off-by: Markus Elfring 
---
 drivers/scsi/aacraid/commctrl.c | 41 -
 1 file changed, 24 insertions(+), 17 deletions(-)

diff --git a/drivers/scsi/aacraid/commctrl.c b/drivers/scsi/aacraid/commctrl.c
index 9f4ddb0..cda03f0 100644
--- a/drivers/scsi/aacraid/commctrl.c
+++ b/drivers/scsi/aacraid/commctrl.c
@@ -251,7 +251,7 @@ static int next_getadapter_fib(struct aac_dev * dev, void 
__user *arg)
struct list_head * entry;
unsigned long flags;
 
-   if(copy_from_user((void *), arg, sizeof(struct fib_ioctl)))
+   if (copy_from_user((void *), arg, sizeof(struct fib_ioctl)))
return -EFAULT;
/*
 *  Verify that the HANDLE passed in was a valid AdapterFibContext
@@ -280,7 +280,7 @@ static int next_getadapter_fib(struct aac_dev * dev, void 
__user *arg)
return -EINVAL;
}
 
-   if((fibctx->type != FSAFS_NTC_GET_ADAPTER_FIB_CONTEXT) ||
+   if ((fibctx->type != FSAFS_NTC_GET_ADAPTER_FIB_CONTEXT) ||
 (fibctx->size != sizeof(struct aac_fib_context))) {
spin_unlock_irqrestore(>fib_lock, flags);
dprintk ((KERN_INFO "Fib Context corrupt?\n"));
@@ -327,7 +327,7 @@ return_fib:
ssleep(1);
}
if (f.wait) {
-   if(down_interruptible(>wait_sem) < 0) {
+   if (down_interruptible(>wait_sem) < 0) {
status = -ERESTARTSYS;
} else {
/* Lock again and retry */
@@ -404,7 +404,7 @@ static int close_getadapter_fib(struct aac_dev * dev, void 
__user *arg)
entry = dev->fib_list.next;
fibctx = NULL;
 
-   while(entry != >fib_list) {
+   while (entry != >fib_list) {
fibctx = list_entry(entry, struct aac_fib_context, next);
/*
 *  Extract the fibctx from the input parameters
@@ -418,7 +418,7 @@ static int close_getadapter_fib(struct aac_dev * dev, void 
__user *arg)
if (!fibctx)
return 0; /* Already gone */
 
-   if((fibctx->type != FSAFS_NTC_GET_ADAPTER_FIB_CONTEXT) ||
+   if ((fibctx->type != FSAFS_NTC_GET_ADAPTER_FIB_CONTEXT) ||
 (fibctx->size != sizeof(struct aac_fib_context)))
return -EINVAL;
spin_lock_irqsave(>fib_lock, flags);
@@ -509,7 +509,7 @@ static int aac_send_raw_srb(struct aac_dev* dev, void 
__user * arg)
srbcmd = (struct aac_srb*) fib_data(srbfib);
 
memset(sg_list, 0, sizeof(sg_list)); /* cleanup may take issue */
-   if(copy_from_user(, _srb->count,sizeof(u32))){
+   if (copy_from_user(, _srb->count, sizeof(u32))) {
dprintk((KERN_DEBUG"aacraid: Could not copy data size from 
user\n"));
rcode = -EFAULT;
goto free_sg_list;
@@ -605,7 +605,7 @@ static int aac_send_raw_srb(struct aac_dev* dev, void 
__user * arg)
}
/* Does this really need to be GFP_DMA? */
p = 
kmalloc(upsg->sg[i].count,GFP_KERNEL|__GFP_DMA);
-   if(!p) {
+   if (!p) {
dprintk((KERN_DEBUG"aacraid: Could not 
allocate SG buffer - size = %d buffer number %d of %d\n",
  upsg->sg[i].count,i,upsg->count));
rcode = -ENOMEM;
@@ -618,7 +618,9 @@ static int aac_send_raw_srb(struct aac_dev* dev, void 
__user * arg)
sg_indx = i;
 
if (flags & SRB_DataOut) {
-   
if(copy_from_user(p,sg_user[i],upsg->sg[i].count)){
+   if (copy_from_user(p,
+  sg_user[i],
+  upsg->sg[i].count)) {
dprintk((KERN_DEBUG"aacraid: 
Could not copy sg data from user\n"));
rcode = -EFAULT;
goto free_user_srbcmd;
@@ -657,7 +659,7 @@ static int aac_send_raw_srb(struct aac_dev* dev, void 
__user * arg)
}
/* Does this really need to be GFP_DMA? */
p = 
kmalloc(usg->sg[i].count,GFP_KERNEL|__GFP_DMA);
-   if(!p) {
+   if (!p) {

[PATCH 4/7] aacraid: Delete unnecessary braces

[SF Markus Elfring]

From: Markus Elfring 
Date: Sun, 21 Aug 2016 07:07:08 +0200

Do not use curly brackets at some source code places
where a single statement should be sufficient.

Signed-off-by: Markus Elfring 
---
 drivers/scsi/aacraid/commctrl.c | 18 ++
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/scsi/aacraid/commctrl.c b/drivers/scsi/aacraid/commctrl.c
index 49a664f..9f4ddb0 100644
--- a/drivers/scsi/aacraid/commctrl.c
+++ b/drivers/scsi/aacraid/commctrl.c
@@ -66,13 +66,11 @@ static int ioctl_send_fib(struct aac_dev * dev, void __user 
*arg)
unsigned int size, osize;
int retval;
 
-   if (dev->in_reset) {
+   if (dev->in_reset)
return -EBUSY;
-   }
fibptr = aac_fib_alloc(dev);
-   if(fibptr == NULL) {
+   if (!fibptr)
return -ENOMEM;
-   }
 
kfib = fibptr->hw_fib_va;
/*
@@ -138,9 +136,8 @@ static int ioctl_send_fib(struct aac_dev * dev, void __user 
*arg)
retval = aac_fib_send(le16_to_cpu(kfib->header.Command), fibptr,
le16_to_cpu(kfib->header.Size) , FsaNormal,
1, 1, NULL, NULL);
-   if (retval) {
+   if (retval)
goto cleanup;
-   }
if (aac_fib_complete(fibptr) != 0) {
retval = -EINVAL;
goto cleanup;
@@ -228,12 +225,10 @@ static int open_getadapter_fib(struct aac_dev * dev, void 
__user *arg)
}
list_add_tail(>next, >fib_list);
spin_unlock_irqrestore(>fib_lock, flags);
-   if (copy_to_user(arg, >unique,
-   sizeof(fibctx->unique))) {
+   if (copy_to_user(arg, >unique, sizeof(fibctx->unique)))
status = -EFAULT;
-   } else {
+   else
status = 0;
-   }
}
return status;
 }
@@ -820,9 +815,8 @@ static int aac_send_raw_srb(struct aac_dev* dev, void 
__user * arg)
 free_user_srbcmd:
kfree(user_srbcmd);
 free_sg_list:
-   for(i=0; i <= sg_indx; i++){
+   for (i = 0; i <= sg_indx; i++)
kfree(sg_list[i]);
-   }
if (rcode != -ERESTARTSYS) {
aac_fib_complete(srbfib);
aac_fib_free(srbfib);
-- 
2.9.3

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

[PATCH 3/7] aacraid: Delete unnecessary initialisations in aac_send_raw_srb()

[SF Markus Elfring]

From: Markus Elfring 
Date: Sat, 20 Aug 2016 21:25:20 +0200

Six local variables will be set to an appropriate value a bit later.
Thus omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring 
---
 drivers/scsi/aacraid/commctrl.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/aacraid/commctrl.c b/drivers/scsi/aacraid/commctrl.c
index 6dcdf91..49a664f 100644
--- a/drivers/scsi/aacraid/commctrl.c
+++ b/drivers/scsi/aacraid/commctrl.c
@@ -476,20 +476,20 @@ static int aac_send_raw_srb(struct aac_dev* dev, void 
__user * arg)
 {
struct fib* srbfib;
int status;
-   struct aac_srb *srbcmd = NULL;
-   struct user_aac_srb *user_srbcmd = NULL;
+   struct aac_srb *srbcmd;
+   struct user_aac_srb *user_srbcmd;
struct user_aac_srb __user *user_srb = arg;
struct aac_srb_reply __user *user_reply;
struct aac_srb_reply* reply;
-   u32 fibsize = 0;
-   u32 flags = 0;
+   u32 fibsize;
+   u32 flags;
s32 rcode = 0;
u32 data_dir;
void __user *sg_user[32];
void *sg_list[32];
u32 sg_indx = 0;
-   u32 byte_count = 0;
-   u32 actual_fibsize64, actual_fibsize = 0;
+   u32 byte_count;
+   u32 actual_fibsize64, actual_fibsize;
int i;
 
 
-- 
2.9.3

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

[PATCH 2/7] aacraid: One function call less in aac_send_raw_srb() after error detection

[SF Markus Elfring]

>From e8187662ee30aab709a260c72fb86c51673f8e0d Mon Sep 17 00:00:00 2001
From: Markus Elfring 
Date: Sat, 20 Aug 2016 20:40:47 +0200
Subject: [PATCH 2/7] aacraid: One function call less in aac_send_raw_srb()
 after error detection

The kfree() function was called in a few cases by the
aac_send_raw_srb() function during error handling
even if the variable "user_srbcmd" contained eventually
an inappropriate pointer value.

Signed-off-by: Markus Elfring 
---
 drivers/scsi/aacraid/commctrl.c | 49 -
 1 file changed, 24 insertions(+), 25 deletions(-)

diff --git a/drivers/scsi/aacraid/commctrl.c b/drivers/scsi/aacraid/commctrl.c
index 1af3084..6dcdf91 100644
--- a/drivers/scsi/aacraid/commctrl.c
+++ b/drivers/scsi/aacraid/commctrl.c
@@ -517,19 +517,19 @@ static int aac_send_raw_srb(struct aac_dev* dev, void 
__user * arg)
if(copy_from_user(, _srb->count,sizeof(u32))){
dprintk((KERN_DEBUG"aacraid: Could not copy data size from 
user\n"));
rcode = -EFAULT;
-   goto cleanup;
+   goto free_sg_list;
}
 
if ((fibsize < (sizeof(struct user_aac_srb) - sizeof(struct 
user_sgentry))) ||
(fibsize > (dev->max_fib_size - sizeof(struct aac_fibhdr {
rcode = -EINVAL;
-   goto cleanup;
+   goto free_sg_list;
}
 
user_srbcmd = memdup_user(user_srb, fibsize);
if (IS_ERR(user_srbcmd)) {
rcode = PTR_ERR(user_srbcmd);
-   goto cleanup;
+   goto free_sg_list;
}
 
user_reply = arg+fibsize;
@@ -564,7 +564,7 @@ static int aac_send_raw_srb(struct aac_dev* dev, void 
__user * arg)
dprintk((KERN_DEBUG"aacraid: too many sg entries %d\n",
  le32_to_cpu(srbcmd->sg.count)));
rcode = -EINVAL;
-   goto cleanup;
+   goto free_user_srbcmd;
}
actual_fibsize = sizeof(struct aac_srb) - sizeof(struct sgentry) +
((user_srbcmd->sg.count & 0xff) * sizeof(struct sgentry));
@@ -580,12 +580,12 @@ static int aac_send_raw_srb(struct aac_dev* dev, void 
__user * arg)
  sizeof(struct aac_srb), sizeof(struct sgentry),
  sizeof(struct sgentry64), fibsize));
rcode = -EINVAL;
-   goto cleanup;
+   goto free_user_srbcmd;
}
if ((data_dir == DMA_NONE) && user_srbcmd->sg.count) {
dprintk((KERN_DEBUG"aacraid: SG with no direction specified in 
Raw SRB command\n"));
rcode = -EINVAL;
-   goto cleanup;
+   goto free_user_srbcmd;
}
byte_count = 0;
if (dev->adapter_info.options & AAC_OPT_SGMAP_HOST64) {
@@ -606,7 +606,7 @@ static int aac_send_raw_srb(struct aac_dev* dev, void 
__user * arg)
  (dev->scsi_host_ptr->max_sectors << 9) :
  65536)) {
rcode = -EINVAL;
-   goto cleanup;
+   goto free_user_srbcmd;
}
/* Does this really need to be GFP_DMA? */
p = 
kmalloc(upsg->sg[i].count,GFP_KERNEL|__GFP_DMA);
@@ -614,7 +614,7 @@ static int aac_send_raw_srb(struct aac_dev* dev, void 
__user * arg)
dprintk((KERN_DEBUG"aacraid: Could not 
allocate SG buffer - size = %d buffer number %d of %d\n",
  upsg->sg[i].count,i,upsg->count));
rcode = -ENOMEM;
-   goto cleanup;
+   goto free_user_srbcmd;
}
addr = (u64)upsg->sg[i].addr[0];
addr += ((u64)upsg->sg[i].addr[1]) << 32;
@@ -626,7 +626,7 @@ static int aac_send_raw_srb(struct aac_dev* dev, void 
__user * arg)

if(copy_from_user(p,sg_user[i],upsg->sg[i].count)){
dprintk((KERN_DEBUG"aacraid: 
Could not copy sg data from user\n"));
rcode = -EFAULT;
-   goto cleanup;
+   goto free_user_srbcmd;
}
}
addr = pci_map_single(dev->pdev, p, 
upsg->sg[i].count, data_dir);
@@ -644,7 +644,7 @@ static int aac_send_raw_srb(struct aac_dev* dev, void 
__user * arg)
if (!usg) {
dprintk((KERN_DEBUG"aacraid: Allocation error 
in Raw SRB 

[PATCH 1/7] aacraid: Use memdup_user() rather than duplicating its implementation

[SF Markus Elfring]

From: Markus Elfring 
Date: Sat, 20 Aug 2016 20:05:24 +0200

Reuse existing functionality from memdup_user() instead of keeping
duplicate source code.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/scsi/aacraid/commctrl.c | 12 +++-
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/scsi/aacraid/commctrl.c b/drivers/scsi/aacraid/commctrl.c
index 5648b71..1af3084 100644
--- a/drivers/scsi/aacraid/commctrl.c
+++ b/drivers/scsi/aacraid/commctrl.c
@@ -526,15 +526,9 @@ static int aac_send_raw_srb(struct aac_dev* dev, void 
__user * arg)
goto cleanup;
}
 
-   user_srbcmd = kmalloc(fibsize, GFP_KERNEL);
-   if (!user_srbcmd) {
-   dprintk((KERN_DEBUG"aacraid: Could not make a copy of the 
srb\n"));
-   rcode = -ENOMEM;
-   goto cleanup;
-   }
-   if(copy_from_user(user_srbcmd, user_srb,fibsize)){
-   dprintk((KERN_DEBUG"aacraid: Could not copy srb from user\n"));
-   rcode = -EFAULT;
+   user_srbcmd = memdup_user(user_srb, fibsize);
+   if (IS_ERR(user_srbcmd)) {
+   rcode = PTR_ERR(user_srbcmd);
goto cleanup;
}
 
-- 
2.9.3

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

[PATCH 0/7] aacraid: Fine-tuning for a few functions

[SF Markus Elfring]

From: Markus Elfring 
Date: Sun, 21 Aug 2016 09:03:21 +0200

Several update suggestions were taken into account
from static source code analysis.

Markus Elfring (7):
  Use memdup_user() rather than duplicating its implementation
  One function call less in aac_send_raw_srb() after error detection
  Delete unnecessary initialisations in aac_send_raw_srb()
  Delete unnecessary braces
  Add spaces after control flow keywords
  Improve determination of a few sizes
  Apply another recommendation from "checkpatch.pl"

 drivers/scsi/aacraid/commctrl.c | 140 +++-
 1 file changed, 67 insertions(+), 73 deletions(-)

-- 
2.9.3

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

[PATCH 4/5] block-cciss: Move an assignment for the variable "sg_used" in cciss_bigpassthru()

[SF Markus Elfring]

From: Markus Elfring 
Date: Wed, 17 Aug 2016 23:04:46 +0200

Move the assignment for the local variable "sg_used" behind the source code
for some memory allocations by this function.

Signed-off-by: Markus Elfring 
---
 drivers/block/cciss.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index 10e1b0a..b08bfb7 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -1575,7 +1575,7 @@ static int cciss_bigpassthru(ctlr_info_t *h, void __user 
*argp)
unsigned char **buff;
int *buff_size;
u64bit temp64;
-   BYTE sg_used = 0;
+   BYTE sg_used;
int status;
int i;
DECLARE_COMPLETION_ONSTACK(wait);
@@ -1616,6 +1616,7 @@ static int cciss_bigpassthru(ctlr_info_t *h, void __user 
*argp)
}
left = ioc->buf_size;
data_ptr = ioc->buf;
+   sg_used = 0;
while (left) {
sz = (left > ioc->malloc_size) ? ioc->malloc_size : left;
buff_size[sg_used] = sz;
-- 
2.9.3

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

[PATCH 5/5] block-cciss: Replace three kzalloc() calls by kcalloc()

[SF Markus Elfring]

From: Markus Elfring 
Date: Thu, 18 Aug 2016 11:26:18 +0200

* The script "checkpatch.pl" can point information out like the following.

  WARNING: Prefer kcalloc over kzalloc with multiply

  Thus fix the affected source code places.

* Replace the specification of data structures by pointer dereferences
  to make the corresponding size determination a bit safer according to
  the Linux coding style convention.

Signed-off-by: Markus Elfring 
---
 drivers/block/cciss.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index b08bfb7..3502a3a 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -1604,12 +1604,12 @@ static int cciss_bigpassthru(ctlr_info_t *h, void 
__user *argp)
status = -EINVAL;
goto free_ioc;
}
-   buff_size = kmalloc(MAXSGENTRIES * sizeof(int), GFP_KERNEL);
+   buff_size = kcalloc(MAXSGENTRIES, sizeof(*buff_size), GFP_KERNEL);
if (!buff_size) {
status = -ENOMEM;
goto free_ioc;
}
-   buff = kzalloc(MAXSGENTRIES * sizeof(char *), GFP_KERNEL);
+   buff = kcalloc(MAXSGENTRIES, sizeof(*buff), GFP_KERNEL);
if (!buff) {
status = -ENOMEM;
goto free_size;
@@ -4838,8 +4838,9 @@ static int cciss_allocate_scatterlists(ctlr_info_t *h)
int i;
 
/* zero it, so that on free we need not know how many were alloc'ed */
-   h->scatter_list = kzalloc(h->max_commands *
-   sizeof(struct scatterlist *), GFP_KERNEL);
+   h->scatter_list = kcalloc(h->max_commands,
+ sizeof(*h->scatter_list),
+ GFP_KERNEL);
if (!h->scatter_list)
return -ENOMEM;
 
-- 
2.9.3

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

[PATCH 3/5] block-cciss: Delete unnecessary initialisations in cciss_bigpassthru()

[SF Markus Elfring]

From: Markus Elfring 
Date: Wed, 17 Aug 2016 22:55:51 +0200

Three local variables will be set to an appropriate value a bit later.
Thus omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring 
---
 drivers/block/cciss.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index 43ac632..10e1b0a 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -1572,11 +1572,11 @@ static int cciss_bigpassthru(ctlr_info_t *h, void 
__user *argp)
 {
BIG_IOCTL_Command_struct *ioc;
CommandList_struct *c;
-   unsigned char **buff = NULL;
-   int *buff_size = NULL;
+   unsigned char **buff;
+   int *buff_size;
u64bit temp64;
BYTE sg_used = 0;
-   int status = 0;
+   int status;
int i;
DECLARE_COMPLETION_ONSTACK(wait);
__u32 left;
-- 
2.9.3

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

[PATCH 2/5] block-cciss: Less function calls in cciss_bigpassthru() after error detection

[SF Markus Elfring]

From: Markus Elfring 
Date: Wed, 17 Aug 2016 22:39:31 +0200

The kfree() function was called in a few cases by the
cciss_bigpassthru() function during error handling
even if a passed variable contained a null pointer.

Adjust jump targets according to the Linux coding style convention.

Signed-off-by: Markus Elfring 
---
 drivers/block/cciss.c | 40 
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index e044342..43ac632 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -1593,26 +1593,26 @@ static int cciss_bigpassthru(ctlr_info_t *h, void 
__user *argp)
if ((ioc->buf_size < 1) &&
(ioc->Request.Type.Direction != XFER_NONE)) {
status = -EINVAL;
-   goto cleanup1;
+   goto free_ioc;
}
/* Check kmalloc limits  using all SGs */
if (ioc->malloc_size > MAX_KMALLOC_SIZE) {
status = -EINVAL;
-   goto cleanup1;
+   goto free_ioc;
}
if (ioc->buf_size > ioc->malloc_size * MAXSGENTRIES) {
status = -EINVAL;
-   goto cleanup1;
-   }
-   buff = kzalloc(MAXSGENTRIES * sizeof(char *), GFP_KERNEL);
-   if (!buff) {
-   status = -ENOMEM;
-   goto cleanup1;
+   goto free_ioc;
}
buff_size = kmalloc(MAXSGENTRIES * sizeof(int), GFP_KERNEL);
if (!buff_size) {
status = -ENOMEM;
-   goto cleanup1;
+   goto free_ioc;
+   }
+   buff = kzalloc(MAXSGENTRIES * sizeof(char *), GFP_KERNEL);
+   if (!buff) {
+   status = -ENOMEM;
+   goto free_size;
}
left = ioc->buf_size;
data_ptr = ioc->buf;
@@ -1622,12 +1622,12 @@ static int cciss_bigpassthru(ctlr_info_t *h, void 
__user *argp)
buff[sg_used] = kmalloc(sz, GFP_KERNEL);
if (buff[sg_used] == NULL) {
status = -ENOMEM;
-   goto cleanup1;
+   goto free_buffer;
}
if (ioc->Request.Type.Direction == XFER_WRITE) {
if (copy_from_user(buff[sg_used], data_ptr, sz)) {
status = -EFAULT;
-   goto cleanup1;
+   goto free_buffer;
}
} else {
memset(buff[sg_used], 0, sz);
@@ -1639,7 +1639,7 @@ static int cciss_bigpassthru(ctlr_info_t *h, void __user 
*argp)
c = cmd_special_alloc(h);
if (!c) {
status = -ENOMEM;
-   goto cleanup1;
+   goto free_buffer;
}
c->cmd_type = CMD_IOCTL_PEND;
c->Header.ReplyQueue = 0;
@@ -1674,7 +1674,7 @@ static int cciss_bigpassthru(ctlr_info_t *h, void __user 
*argp)
if (copy_to_user(argp, ioc, sizeof(*ioc))) {
cmd_special_free(h, c);
status = -EFAULT;
-   goto cleanup1;
+   goto free_buffer;
}
if (ioc->Request.Type.Direction == XFER_READ) {
/* Copy the data out of the buffer we created */
@@ -1683,20 +1683,20 @@ static int cciss_bigpassthru(ctlr_info_t *h, void 
__user *argp)
if (copy_to_user(ptr, buff[i], buff_size[i])) {
cmd_special_free(h, c);
status = -EFAULT;
-   goto cleanup1;
+   goto free_buffer;
}
ptr += buff_size[i];
}
}
cmd_special_free(h, c);
status = 0;
-cleanup1:
-   if (buff) {
-   for (i = 0; i < sg_used; i++)
-   kfree(buff[i]);
-   kfree(buff);
-   }
+free_buffer:
+   for (i = 0; i < sg_used; i++)
+   kfree(buff[i]);
+   kfree(buff);
+free_size:
kfree(buff_size);
+free_ioc:
kfree(ioc);
return status;
 }
-- 
2.9.3

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

[PATCH 0/5] block-cciss: Fine-tuning for two function implementations

[SF Markus Elfring]

From: Markus Elfring 
Date: Thu, 18 Aug 2016 11:40:04 +0200

Some update suggestions were taken into account
from static source code analysis.

Markus Elfring (5):
  Use memdup_user()
  Less function calls after error detection
  Delete unnecessary initialisations
  Move an assignment for the variable "sg_used"
  Replace three kzalloc() calls by kcalloc()

 drivers/block/cciss.c | 66 ---
 1 file changed, 31 insertions(+), 35 deletions(-)

-- 
2.9.3

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

[PATCH 1/5] block-cciss: Use memdup_user() rather than duplicating its implementation

[SF Markus Elfring]

From: Markus Elfring 
Date: Wed, 17 Aug 2016 22:10:29 +0200

* Reuse existing functionality from memdup_user() instead of keeping
  duplicate source code.

  This issue was detected by using the Coccinelle software.

* Return directly if this copy operation failed.

Signed-off-by: Markus Elfring 
---
 drivers/block/cciss.c | 12 +++-
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index db9d6bb..e044342 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -1587,15 +1587,9 @@ static int cciss_bigpassthru(ctlr_info_t *h, void __user 
*argp)
return -EINVAL;
if (!capable(CAP_SYS_RAWIO))
return -EPERM;
-   ioc = kmalloc(sizeof(*ioc), GFP_KERNEL);
-   if (!ioc) {
-   status = -ENOMEM;
-   goto cleanup1;
-   }
-   if (copy_from_user(ioc, argp, sizeof(*ioc))) {
-   status = -EFAULT;
-   goto cleanup1;
-   }
+   ioc = memdup_user(argp, sizeof(*ioc));
+   if (IS_ERR(ioc))
+   return PTR_ERR(ioc);
if ((ioc->buf_size < 1) &&
(ioc->Request.Type.Direction != XFER_NONE)) {
status = -EINVAL;
-- 
2.9.3

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

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

[SF Markus Elfring]

From: Markus Elfring 
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 
---
 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(>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 majord...@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 
> 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 
> ---
>  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
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[SF Markus Elfring]

From: Markus Elfring 
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 
---
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 = _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

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

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

[SF Markus Elfring]

From: Markus Elfring 
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ß 
Signed-off-by: Markus Elfring 
---
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_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_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(_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

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

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

[SF Markus Elfring]

From: Markus Elfring 
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 
---
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(_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

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

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

[SF Markus Elfring]

From: Markus Elfring 
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

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

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
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[SF Markus Elfring]

From: Markus Elfring 
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 
---
 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 = _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

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

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

[SF Markus Elfring]

From: Markus Elfring 
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 
---
 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_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_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(_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

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

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

[SF Markus Elfring]

From: Markus Elfring 
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 
---
 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(_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

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

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

[SF Markus Elfring]

From: Markus Elfring 
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

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

[PATCH] SCSI-lpfc: Use a signed return type for two functions

[SF Markus Elfring]

From: Markus Elfring 
Date: Sat, 19 Dec 2015 19:32:27 +0100

The return type "size_t" was used by the functions "lpfc_wwn_set"
and "lpfc_oas_lun_state_set" despite of the aspect that they will
eventually return a negative error code.
Improve this implementation detail by using the type "ssize_t" instead.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/scsi/lpfc/lpfc_attr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_attr.c b/drivers/scsi/lpfc/lpfc_attr.c
index f6446d7..777960e 100644
--- a/drivers/scsi/lpfc/lpfc_attr.c
+++ b/drivers/scsi/lpfc/lpfc_attr.c
@@ -2089,7 +2089,7 @@ static char *lpfc_soft_wwn_key = "C99G71SL8032A";
  * -EINVAL if the buffer does not contain a valid wwn
  * 0 success
  **/
-static size_t
+static ssize_t
 lpfc_wwn_set(const char *buf, size_t cnt, char wwn[])
 {
unsigned int i, j;
@@ -2570,7 +2570,7 @@ static DEVICE_ATTR(lpfc_xlane_lun_status, S_IRUGO,
  * -EPERM OAS is not enabled or not supported by this port.
  *
  */
-static size_t
+static ssize_t
 lpfc_oas_lun_state_set(struct lpfc_hba *phba, uint8_t vpt_wwpn[],
   uint8_t tgt_wwpn[], uint64_t lun, uint32_t oas_state)
 {
-- 
2.6.3

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

Re: [PATCH 4/7] iscsi-target: Make a variable initialisation a bit more obvious in iscsi_create_default_params()

[SF Markus Elfring]

>> @@ -200,9 +200,8 @@ free_param:
>>  int iscsi_create_default_params(struct iscsi_param_list **param_list_ptr)
>>  {
>>  struct iscsi_param *param;
>> -struct iscsi_param_list *pl;
>> +struct iscsi_param_list *pl = kzalloc(sizeof(*pl), GFP_KERNEL);
>>
>> -pl = kzalloc(sizeof(struct iscsi_param_list), GFP_KERNEL);
> 
> I don't see the benefit of this change, and the pattern assignment ->
> failure test becomes more obscure.

Are there any more software developers who prefer to specify
such a variable initialisation on a single line?

Does the proposed small source code reduction matter for you?

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

[PATCH 0/7] iSCSI-target: Fine-tuning for three function implementations

[SF Markus Elfring]

From: Markus Elfring 
Date: Sat, 12 Dec 2015 15:25:20 +0100

Some update suggestions were taken into account from static
source code analysis.

Markus Elfring (7):
  Use a variable initialisation in iscsi_set_default_param() directly
  Less checks in iscsi_set_default_param() after error detection
  Delete an unnecessary variable initialisation in iscsi_create_default_params()
  Make a variable initialisation a bit more obvious in 
iscsi_create_default_params()
  Rename a jump label in iscsi_create_default_params()
  Delete unnecessary variable initialisations in 
iscsi_check_valuelist_for_support()
  Make two variable initialisations a bit more obvious in 
iscsi_check_valuelist_for_support()

 drivers/target/iscsi/iscsi_target_parameters.c | 100 -
 1 file changed, 47 insertions(+), 53 deletions(-)

-- 
2.6.3

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

[PATCH 7/7] iscsi-target: Make two variable initialisations a bit more obvious in iscsi_check_valuelist_for_support()

[SF Markus Elfring]

From: Markus Elfring 
Date: Sat, 12 Dec 2015 15:04:57 +0100

The variable "acceptor_values" and "proposer_values" were initialized
by null pointers and immediately assigned values from input parameters
by separate statements.
Let us express the desired variable initialisations directly.

Signed-off-by: Markus Elfring 
---
 drivers/target/iscsi/iscsi_target_parameters.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target_parameters.c 
b/drivers/target/iscsi/iscsi_target_parameters.c
index 53e3345..fb6fd34 100644
--- a/drivers/target/iscsi/iscsi_target_parameters.c
+++ b/drivers/target/iscsi/iscsi_target_parameters.c
@@ -921,10 +921,7 @@ static char *iscsi_check_valuelist_for_support(
char *value)
 {
char *tmp1, *tmp2;
-   char *acceptor_values = NULL, *proposer_values = NULL;
-
-   acceptor_values = param->value;
-   proposer_values = value;
+   char *acceptor_values = param->value, *proposer_values = value;
 
do {
if (!proposer_values)
-- 
2.6.3

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

[PATCH 6/7] iscsi-target: Delete unnecessary variable initialisations in iscsi_check_valuelist_for_support()

[SF Markus Elfring]

From: Markus Elfring 
Date: Sat, 12 Dec 2015 14:34:26 +0100

The variables "tmp1" and "tmp2" will eventually be set to appropriate
pointers from a call of the strchr() function.
Thus let us omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring 
---
 drivers/target/iscsi/iscsi_target_parameters.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/target/iscsi/iscsi_target_parameters.c 
b/drivers/target/iscsi/iscsi_target_parameters.c
index 29ecf29..53e3345 100644
--- a/drivers/target/iscsi/iscsi_target_parameters.c
+++ b/drivers/target/iscsi/iscsi_target_parameters.c
@@ -920,7 +920,7 @@ static char *iscsi_check_valuelist_for_support(
struct iscsi_param *param,
char *value)
 {
-   char *tmp1 = NULL, *tmp2 = NULL;
+   char *tmp1, *tmp2;
char *acceptor_values = NULL, *proposer_values = NULL;
 
acceptor_values = param->value;
-- 
2.6.3

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

[PATCH 1/7] iscsi-target: Use a variable initialisation in iscsi_set_default_param() directly

[SF Markus Elfring]

From: Markus Elfring 
Date: Sat, 12 Dec 2015 11:36:02 +0100

Omit the unnecessary setting to a null pointer for the variable "param"
at the beginning of the function "iscsi_set_default_param"
because it can be directly initialized with the return value
from the function "kzalloc".

Signed-off-by: Markus Elfring 
---
 drivers/target/iscsi/iscsi_target_parameters.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target_parameters.c 
b/drivers/target/iscsi/iscsi_target_parameters.c
index 3a1f9a7..0a8bd3f 100644
--- a/drivers/target/iscsi/iscsi_target_parameters.c
+++ b/drivers/target/iscsi/iscsi_target_parameters.c
@@ -127,9 +127,8 @@ static struct iscsi_param *iscsi_set_default_param(struct 
iscsi_param_list *para
char *name, char *value, u8 phase, u8 scope, u8 sender,
u16 type_range, u8 use)
 {
-   struct iscsi_param *param = NULL;
+   struct iscsi_param *param = kzalloc(sizeof(*param), GFP_KERNEL);
 
-   param = kzalloc(sizeof(struct iscsi_param), GFP_KERNEL);
if (!param) {
pr_err("Unable to allocate memory for parameter.\n");
goto out;
-- 
2.6.3

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

[PATCH 2/7] iscsi-target: Less checks in iscsi_set_default_param() after error detection

[SF Markus Elfring]

From: Markus Elfring 
Date: Sat, 12 Dec 2015 12:50:10 +0100

This issue was detected by using the Coccinelle software.

A sanity check would be performed by the iscsi_set_default_param() function
even if it is known already that the passed variable contained
a null pointer.

* This implementation detail could be improved by adjustments
  for jump targets according to the Linux coding style convention.

* Let us return directly if a call of the function "kzalloc" failed.

Signed-off-by: Markus Elfring 
---
 drivers/target/iscsi/iscsi_target_parameters.c | 21 ++---
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target_parameters.c 
b/drivers/target/iscsi/iscsi_target_parameters.c
index 0a8bd3f..15b2618 100644
--- a/drivers/target/iscsi/iscsi_target_parameters.c
+++ b/drivers/target/iscsi/iscsi_target_parameters.c
@@ -131,20 +131,20 @@ static struct iscsi_param *iscsi_set_default_param(struct 
iscsi_param_list *para
 
if (!param) {
pr_err("Unable to allocate memory for parameter.\n");
-   goto out;
+   return NULL;
}
INIT_LIST_HEAD(>p_list);
 
param->name = kstrdup(name, GFP_KERNEL);
if (!param->name) {
pr_err("Unable to allocate memory for parameter name.\n");
-   goto out;
+   goto free_param;
}
 
param->value = kstrdup(value, GFP_KERNEL);
if (!param->value) {
pr_err("Unable to allocate memory for parameter value.\n");
-   goto out;
+   goto free_name;
}
 
param->phase= phase;
@@ -182,18 +182,17 @@ static struct iscsi_param *iscsi_set_default_param(struct 
iscsi_param_list *para
default:
pr_err("Unknown type_range 0x%02x\n",
param->type_range);
-   goto out;
+   goto free_value;
}
list_add_tail(>p_list, _list->param_list);
 
return param;
-out:
-   if (param) {
-   kfree(param->value);
-   kfree(param->name);
-   kfree(param);
-   }
-
+free_value:
+   kfree(param->value);
+free_name:
+   kfree(param->name);
+free_param:
+   kfree(param);
return NULL;
 }
 
-- 
2.6.3

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

[PATCH 4/7] iscsi-target: Make a variable initialisation a bit more obvious in iscsi_create_default_params()

[SF Markus Elfring]

From: Markus Elfring 
Date: Sat, 12 Dec 2015 13:44:06 +0100

The variable "pl" was declared and immediately assigned a return value
from a function call in a separate statement.

* Let us express the desired variable initialisation directly.

* Avoid the repetition of the data type specification for the
  involved memory allocation according to the Linux coding
  style convention.

Signed-off-by: Markus Elfring 
---
 drivers/target/iscsi/iscsi_target_parameters.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target_parameters.c 
b/drivers/target/iscsi/iscsi_target_parameters.c
index e0b173d..3f3842f 100644
--- a/drivers/target/iscsi/iscsi_target_parameters.c
+++ b/drivers/target/iscsi/iscsi_target_parameters.c
@@ -200,9 +200,8 @@ free_param:
 int iscsi_create_default_params(struct iscsi_param_list **param_list_ptr)
 {
struct iscsi_param *param;
-   struct iscsi_param_list *pl;
+   struct iscsi_param_list *pl = kzalloc(sizeof(*pl), GFP_KERNEL);
 
-   pl = kzalloc(sizeof(struct iscsi_param_list), GFP_KERNEL);
if (!pl) {
pr_err("Unable to allocate memory for"
" struct iscsi_param_list.\n");
-- 
2.6.3

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

[PATCH 5/7] iscsi-target: Rename a jump label in iscsi_create_default_params()

[SF Markus Elfring]

From: Markus Elfring 
Date: Sat, 12 Dec 2015 14:12:50 +0100

This issue was detected by using the Coccinelle software.

Choose a jump label according to the current Linux coding style convention.

Signed-off-by: Markus Elfring 
---
 drivers/target/iscsi/iscsi_target_parameters.c | 64 +-
 1 file changed, 32 insertions(+), 32 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target_parameters.c 
b/drivers/target/iscsi/iscsi_target_parameters.c
index 3f3842f..29ecf29 100644
--- a/drivers/target/iscsi/iscsi_target_parameters.c
+++ b/drivers/target/iscsi/iscsi_target_parameters.c
@@ -225,185 +225,185 @@ int iscsi_create_default_params(struct iscsi_param_list 
**param_list_ptr)
PHASE_SECURITY, SCOPE_CONNECTION_ONLY, SENDER_BOTH,
TYPERANGE_AUTH, USE_INITIAL_ONLY);
if (!param)
-   goto out;
+   goto release_list;
 
param = iscsi_set_default_param(pl, HEADERDIGEST, INITIAL_HEADERDIGEST,
PHASE_OPERATIONAL, SCOPE_CONNECTION_ONLY, SENDER_BOTH,
TYPERANGE_DIGEST, USE_INITIAL_ONLY);
if (!param)
-   goto out;
+   goto release_list;
 
param = iscsi_set_default_param(pl, DATADIGEST, INITIAL_DATADIGEST,
PHASE_OPERATIONAL, SCOPE_CONNECTION_ONLY, SENDER_BOTH,
TYPERANGE_DIGEST, USE_INITIAL_ONLY);
if (!param)
-   goto out;
+   goto release_list;
 
param = iscsi_set_default_param(pl, MAXCONNECTIONS,
INITIAL_MAXCONNECTIONS, PHASE_OPERATIONAL,
SCOPE_SESSION_WIDE, SENDER_BOTH,
TYPERANGE_1_TO_65535, USE_LEADING_ONLY);
if (!param)
-   goto out;
+   goto release_list;
 
param = iscsi_set_default_param(pl, SENDTARGETS, INITIAL_SENDTARGETS,
PHASE_FFP0, SCOPE_SESSION_WIDE, SENDER_INITIATOR,
TYPERANGE_UTF8, 0);
if (!param)
-   goto out;
+   goto release_list;
 
param = iscsi_set_default_param(pl, TARGETNAME, INITIAL_TARGETNAME,
PHASE_DECLARATIVE, SCOPE_SESSION_WIDE, SENDER_BOTH,
TYPERANGE_ISCSINAME, USE_ALL);
if (!param)
-   goto out;
+   goto release_list;
 
param = iscsi_set_default_param(pl, INITIATORNAME,
INITIAL_INITIATORNAME, PHASE_DECLARATIVE,
SCOPE_SESSION_WIDE, SENDER_INITIATOR,
TYPERANGE_ISCSINAME, USE_INITIAL_ONLY);
if (!param)
-   goto out;
+   goto release_list;
 
param = iscsi_set_default_param(pl, TARGETALIAS, INITIAL_TARGETALIAS,
PHASE_DECLARATIVE, SCOPE_SESSION_WIDE, SENDER_TARGET,
TYPERANGE_UTF8, USE_ALL);
if (!param)
-   goto out;
+   goto release_list;
 
param = iscsi_set_default_param(pl, INITIATORALIAS,
INITIAL_INITIATORALIAS, PHASE_DECLARATIVE,
SCOPE_SESSION_WIDE, SENDER_INITIATOR, TYPERANGE_UTF8,
USE_ALL);
if (!param)
-   goto out;
+   goto release_list;
 
param = iscsi_set_default_param(pl, TARGETADDRESS,
INITIAL_TARGETADDRESS, PHASE_DECLARATIVE,
SCOPE_SESSION_WIDE, SENDER_TARGET,
TYPERANGE_TARGETADDRESS, USE_ALL);
if (!param)
-   goto out;
+   goto release_list;
 
param = iscsi_set_default_param(pl, TARGETPORTALGROUPTAG,
INITIAL_TARGETPORTALGROUPTAG,
PHASE_DECLARATIVE, SCOPE_SESSION_WIDE, SENDER_TARGET,
TYPERANGE_0_TO_65535, USE_INITIAL_ONLY);
if (!param)
-   goto out;
+   goto release_list;
 
param = iscsi_set_default_param(pl, INITIALR2T, INITIAL_INITIALR2T,
PHASE_OPERATIONAL, SCOPE_SESSION_WIDE, SENDER_BOTH,
TYPERANGE_BOOL_OR, USE_LEADING_ONLY);
if (!param)
-   goto out;
+   goto release_list;
 
param = iscsi_set_default_param(pl, IMMEDIATEDATA,
INITIAL_IMMEDIATEDATA, PHASE_OPERATIONAL,
SCOPE_SESSION_WIDE, SENDER_BOTH, TYPERANGE_BOOL_AND,
USE_LEADING_ONLY);
if (!param)
-   goto out;
+   goto release_list;
 
param = iscsi_set_default_param(pl, MAXXMITDATASEGMENTLENGTH,
INITIAL_MAXXMITDATASEGMENTLENGTH,
PHASE_OPERATIONAL, SCOPE_CONNECTION_ONLY, SENDER_BOTH,
TYPERANGE_512_TO_16777215, 

[PATCH 3/7] iscsi-target: Delete an unnecessary variable initialisation in iscsi_create_default_params()

[SF Markus Elfring]

From: Markus Elfring 
Date: Sat, 12 Dec 2015 13:20:08 +0100

The variable "param" will eventually be set to an appropriate pointer
from a call of the iscsi_set_default_param() function.
Thus let us omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring 
---
 drivers/target/iscsi/iscsi_target_parameters.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/target/iscsi/iscsi_target_parameters.c 
b/drivers/target/iscsi/iscsi_target_parameters.c
index 15b2618..e0b173d 100644
--- a/drivers/target/iscsi/iscsi_target_parameters.c
+++ b/drivers/target/iscsi/iscsi_target_parameters.c
@@ -199,7 +199,7 @@ free_param:
 /* #warning Add extension keys */
 int iscsi_create_default_params(struct iscsi_param_list **param_list_ptr)
 {
-   struct iscsi_param *param = NULL;
+   struct iscsi_param *param;
struct iscsi_param_list *pl;
 
pl = kzalloc(sizeof(struct iscsi_param_list), GFP_KERNEL);
-- 
2.6.3

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