linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: SF Markus Elfring <elfring@users.sourceforge.net>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: linux-scsi@vger.kernel.org, target-devel@vger.kernel.org,
	"Nicholas A. Bellinger" <nab@linux-iscsi.org>,
	LKML <linux-kernel@vger.kernel.org>,
	kernel-janitors@vger.kernel.org,
	Julia Lawall <julia.lawall@lip6.fr>
Subject: Re: [PATCH 1/7] iscsi-target: Use a variable initialisation in iscsi_set_default_param() directly
Date: Sat, 12 Dec 2015 22:22:55 +0100	[thread overview]
Message-ID: <566C902F.8070802@users.sourceforge.net> (raw)
In-Reply-To: <20151212194939.GQ5284@mwanda>

>> @@ -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;
> 
> It's better to just get rid of the initialization but leave the
> kzalloc() as-is for two reasons.
> 
> 1)  Initializer code normally contains more bugs per line than other
>     code.  I am thinking about dereferencing pointers before checking
>     for NULL or not checking the allocation for failure.

I can follow your concerns a bit.


> 2)  It puts a blank line between the allocation and the check for failure.

Is there a target conflict between "convenient" variable initialisation
in the declaration section and the function outline that seems to be checked
by the script "checkpatch.pl" to some degree while corresponding preferences
or recommendations are not mentioned in the document "CodingStyle"?


>     It's like a new paragraph.

I do not see the separation in a strict way so far.


>     The allocation and the check should be next to each other.

I find that these actions are still close enough in the discussed use case.

Regards,
Markus

  reply	other threads:[~2015-12-12 21:22 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <566ABCD9.1060404@users.sourceforge.net>
2015-12-12 14:30 ` [PATCH 0/7] iSCSI-target: Fine-tuning for three function implementations SF Markus Elfring
2015-12-12 14:34   ` [PATCH 1/7] iscsi-target: Use a variable initialisation in iscsi_set_default_param() directly SF Markus Elfring
2015-12-12 19:49     ` Dan Carpenter
2015-12-12 21:22       ` SF Markus Elfring [this message]
2015-12-14  8:41       ` Johannes Thumshirn
2015-12-14 11:38         ` SF Markus Elfring
2015-12-12 14:37   ` [PATCH 2/7] iscsi-target: Less checks in iscsi_set_default_param() after error detection SF Markus Elfring
2015-12-12 14:40   ` [PATCH 3/7] iscsi-target: Delete an unnecessary variable initialisation in iscsi_create_default_params() SF Markus Elfring
2015-12-12 14:41   ` [PATCH 4/7] iscsi-target: Make a variable initialisation a bit more obvious " SF Markus Elfring
2015-12-12 14:45     ` Julia Lawall
2015-12-12 15:02       ` SF Markus Elfring
2015-12-12 14:42   ` [PATCH 5/7] iscsi-target: Rename a jump label " SF Markus Elfring
2015-12-12 14:43   ` [PATCH 6/7] iscsi-target: Delete unnecessary variable initialisations in iscsi_check_valuelist_for_support() SF Markus Elfring
2015-12-12 14:45   ` [PATCH 7/7] iscsi-target: Make two variable initialisations a bit more obvious " SF Markus Elfring
2015-12-12 17:17     ` walter harms
2016-08-18  9:48 ` [PATCH 0/5] block-cciss: Fine-tuning for two function implementations SF Markus Elfring
2016-08-18  9:55   ` [PATCH 1/5] block-cciss: Use memdup_user() rather than duplicating its implementation SF Markus Elfring
2016-08-18  9:56   ` [PATCH 2/5] block-cciss: Less function calls in cciss_bigpassthru() after error detection SF Markus Elfring
2016-08-18 10:00   ` [PATCH 3/5] block-cciss: Delete unnecessary initialisations in cciss_bigpassthru() SF Markus Elfring
2016-08-18 10:02   ` [PATCH 4/5] block-cciss: Move an assignment for the variable "sg_used" " SF Markus Elfring
2016-08-18 10:03   ` [PATCH 5/5] block-cciss: Replace three kzalloc() calls by kcalloc() SF Markus Elfring
2017-08-06 15:00   ` [PATCH 0/5] block-cciss: Fine-tuning for two function implementations SF Markus Elfring
2016-08-21  7:14 ` [PATCH 0/7] aacraid: Fine-tuning for a few functions SF Markus Elfring
2016-08-21  7:19   ` [PATCH 1/7] aacraid: Use memdup_user() rather than duplicating its implementation SF Markus Elfring
2016-08-22 18:00     ` David Carroll
2016-08-22 20:23       ` SF Markus Elfring
2016-08-24 23:01         ` David Carroll
2016-08-21  7:20   ` [PATCH 2/7] aacraid: One function call less in aac_send_raw_srb() after error detection SF Markus Elfring
2016-08-21  7:22   ` [PATCH 3/7] aacraid: Delete unnecessary initialisations in aac_send_raw_srb() SF Markus Elfring
2016-08-21  7:24   ` [PATCH 4/7] aacraid: Delete unnecessary braces SF Markus Elfring
2016-08-21  7:25   ` [PATCH 5/7] aacraid: Add spaces after control flow keywords SF Markus Elfring
2016-08-21  7:27   ` [PATCH 6/7] aacraid: Improve determination of a few sizes SF Markus Elfring
2016-08-21  7:29   ` [PATCH 7/7] aacraid: Apply another recommendation from "checkpatch.pl" SF Markus Elfring
2016-08-21  8:48 ` [PATCH] megaraid_sas: Use memdup_user() rather than duplicating its implementation SF Markus Elfring
2016-08-22  9:31   ` Sumit Saxena
2016-08-24  2:47   ` Martin K. Petersen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=566C902F.8070802@users.sourceforge.net \
    --to=elfring@users.sourceforge.net \
    --cc=dan.carpenter@oracle.com \
    --cc=julia.lawall@lip6.fr \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=nab@linux-iscsi.org \
    --cc=target-devel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).