[PATCH 1/1] staging: unisys: Remove allocation from declaration line

[PATCH 1/1] staging: unisys: Remove allocation from declaration line

[Quentin Lambert]

This patch removes allocation from declaration line because
people are known to gloss over declarations.

Signed-off-by: Quentin Lambert <lambert.quentin@gmail.com>
---
 drivers/staging/unisys/visorchipset/visorchipset_main.c | 6 +++---
 drivers/staging/unisys/visorutil/charqueue.c            | 3 ++-
 drivers/staging/unisys/visorutil/memregion_direct.c     | 5 +++--
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/unisys/visorchipset/visorchipset_main.c b/drivers/staging/unisys/visorchipset/visorchipset_main.c
index 82e259d..a6c6bb7 100644
--- a/drivers/staging/unisys/visorchipset/visorchipset_main.c
+++ b/drivers/staging/unisys/visorchipset/visorchipset_main.c
@@ -1604,9 +1604,9 @@ parahotplug_next_expiration(void)
 static struct parahotplug_request *
 parahotplug_request_create(struct controlvm_message *msg)
 {
-	struct parahotplug_request *req =
-	    kmalloc(sizeof(struct parahotplug_request),
-		    GFP_KERNEL|__GFP_NORETRY);
+	struct parahotplug_request *req;
+
+	req = kmalloc(sizeof(struct parahotplug_request), GFP_KERNEL|__GFP_NORETRY);
 	if (req == NULL)
 		return NULL;
 
diff --git a/drivers/staging/unisys/visorutil/charqueue.c b/drivers/staging/unisys/visorutil/charqueue.c
index ac7acb7..e2ee5ee 100644
--- a/drivers/staging/unisys/visorutil/charqueue.c
+++ b/drivers/staging/unisys/visorutil/charqueue.c
@@ -36,8 +36,9 @@ struct charqueue {
 struct charqueue *visor_charqueue_create(ulong nslots)
 {
 	int alloc_size = sizeof(struct charqueue) + nslots + 1;
-	struct charqueue *cq = kmalloc(alloc_size, GFP_KERNEL|__GFP_NORETRY);
+	struct charqueue *cq;
 
+	cq = kmalloc(alloc_size, GFP_KERNEL|__GFP_NORETRY);
 	if (cq == NULL) {
 		ERRDRV("visor_charqueue_create allocation failed (alloc_size=%d)",
 		       alloc_size);
diff --git a/drivers/staging/unisys/visorutil/memregion_direct.c b/drivers/staging/unisys/visorutil/memregion_direct.c
index 33522cc..f4ecac0 100644
--- a/drivers/staging/unisys/visorutil/memregion_direct.c
+++ b/drivers/staging/unisys/visorutil/memregion_direct.c
@@ -41,8 +41,9 @@ struct memregion *
 visor_memregion_create(HOSTADDRESS physaddr, ulong nbytes)
 {
 	struct memregion *rc = NULL;
-	struct memregion *memregion = kzalloc(sizeof(*memregion),
-					      GFP_KERNEL | __GFP_NORETRY);
+	struct memregion *memregion;
+
+	memregion = kzalloc(sizeof(*memregion), GFP_KERNEL | __GFP_NORETRY);
 	if (memregion == NULL) {
 		ERRDRV("visor_memregion_create allocation failed");
 		return NULL;
-- 
1.9.1

[PATCH 1/1] staging: unisys: Remove allocation from declaration line

[Quentin Lambert]

This patch removes allocation from declaration line because
people are known to gloss over declarations.

Signed-off-by: Quentin Lambert <lambert.quentin@gmail.com>
---
 drivers/staging/unisys/visorchipset/visorchipset_main.c | 6 +++---
 drivers/staging/unisys/visorutil/charqueue.c            | 3 ++-
 drivers/staging/unisys/visorutil/memregion_direct.c     | 5 +++--
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/unisys/visorchipset/visorchipset_main.c b/drivers/staging/unisys/visorchipset/visorchipset_main.c
index 82e259d..a6c6bb7 100644
--- a/drivers/staging/unisys/visorchipset/visorchipset_main.c
+++ b/drivers/staging/unisys/visorchipset/visorchipset_main.c
@@ -1604,9 +1604,9 @@ parahotplug_next_expiration(void)
 static struct parahotplug_request *
 parahotplug_request_create(struct controlvm_message *msg)
 {
-	struct parahotplug_request *req -	    kmalloc(sizeof(struct parahotplug_request),
-		    GFP_KERNEL|__GFP_NORETRY);
+	struct parahotplug_request *req;
+
+	req = kmalloc(sizeof(struct parahotplug_request), GFP_KERNEL|__GFP_NORETRY);
 	if (req = NULL)
 		return NULL;
 
diff --git a/drivers/staging/unisys/visorutil/charqueue.c b/drivers/staging/unisys/visorutil/charqueue.c
index ac7acb7..e2ee5ee 100644
--- a/drivers/staging/unisys/visorutil/charqueue.c
+++ b/drivers/staging/unisys/visorutil/charqueue.c
@@ -36,8 +36,9 @@ struct charqueue {
 struct charqueue *visor_charqueue_create(ulong nslots)
 {
 	int alloc_size = sizeof(struct charqueue) + nslots + 1;
-	struct charqueue *cq = kmalloc(alloc_size, GFP_KERNEL|__GFP_NORETRY);
+	struct charqueue *cq;
 
+	cq = kmalloc(alloc_size, GFP_KERNEL|__GFP_NORETRY);
 	if (cq = NULL) {
 		ERRDRV("visor_charqueue_create allocation failed (alloc_size=%d)",
 		       alloc_size);
diff --git a/drivers/staging/unisys/visorutil/memregion_direct.c b/drivers/staging/unisys/visorutil/memregion_direct.c
index 33522cc..f4ecac0 100644
--- a/drivers/staging/unisys/visorutil/memregion_direct.c
+++ b/drivers/staging/unisys/visorutil/memregion_direct.c
@@ -41,8 +41,9 @@ struct memregion *
 visor_memregion_create(HOSTADDRESS physaddr, ulong nbytes)
 {
 	struct memregion *rc = NULL;
-	struct memregion *memregion = kzalloc(sizeof(*memregion),
-					      GFP_KERNEL | __GFP_NORETRY);
+	struct memregion *memregion;
+
+	memregion = kzalloc(sizeof(*memregion), GFP_KERNEL | __GFP_NORETRY);
 	if (memregion = NULL) {
 		ERRDRV("visor_memregion_create allocation failed");
 		return NULL;
-- 
1.9.1

Re: [PATCH 1/1] staging: unisys: Remove allocation from declaration line

[Sudip Mukherjee]

On Tue, Feb 10, 2015 at 02:02:14PM +0100, Quentin Lambert wrote:
> This patch removes allocation from declaration line because
> people are known to gloss over declarations.
> 
> Signed-off-by: Quentin Lambert <lambert.quentin@gmail.com>
> ---
>  drivers/staging/unisys/visorchipset/visorchipset_main.c | 6 +++---
>  drivers/staging/unisys/visorutil/charqueue.c            | 3 ++-
>  drivers/staging/unisys/visorutil/memregion_direct.c     | 5 +++--
>  3 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/staging/unisys/visorchipset/visorchipset_main.c b/drivers/staging/unisys/visorchipset/visorchipset_main.c
> index 82e259d..a6c6bb7 100644
> --- a/drivers/staging/unisys/visorchipset/visorchipset_main.c
> +++ b/drivers/staging/unisys/visorchipset/visorchipset_main.c
> @@ -1604,9 +1604,9 @@ parahotplug_next_expiration(void)
>  static struct parahotplug_request *
>  parahotplug_request_create(struct controlvm_message *msg)
>  {
> -	struct parahotplug_request *req =
> -	    kmalloc(sizeof(struct parahotplug_request),
> -		    GFP_KERNEL|__GFP_NORETRY);
> +	struct parahotplug_request *req;
> +
> +	req = kmalloc(sizeof(struct parahotplug_request), GFP_KERNEL|__GFP_NORETRY);
if you make it as:
	req = kmalloc(sizeof(*req), GFP_KERNEL|__GFP_NORETRY);
then this will not give you checkpatch warning of line over 80 char.

regards
sudip

>  	if (req == NULL)
>  		return NULL;
<snip>	
>  		return NULL;
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] staging: unisys: Remove allocation from declaration line

[Sudip Mukherjee]

On Tue, Feb 10, 2015 at 02:02:14PM +0100, Quentin Lambert wrote:
> This patch removes allocation from declaration line because
> people are known to gloss over declarations.
> 
> Signed-off-by: Quentin Lambert <lambert.quentin@gmail.com>
> ---
>  drivers/staging/unisys/visorchipset/visorchipset_main.c | 6 +++---
>  drivers/staging/unisys/visorutil/charqueue.c            | 3 ++-
>  drivers/staging/unisys/visorutil/memregion_direct.c     | 5 +++--
>  3 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/staging/unisys/visorchipset/visorchipset_main.c b/drivers/staging/unisys/visorchipset/visorchipset_main.c
> index 82e259d..a6c6bb7 100644
> --- a/drivers/staging/unisys/visorchipset/visorchipset_main.c
> +++ b/drivers/staging/unisys/visorchipset/visorchipset_main.c
> @@ -1604,9 +1604,9 @@ parahotplug_next_expiration(void)
>  static struct parahotplug_request *
>  parahotplug_request_create(struct controlvm_message *msg)
>  {
> -	struct parahotplug_request *req > -	    kmalloc(sizeof(struct parahotplug_request),
> -		    GFP_KERNEL|__GFP_NORETRY);
> +	struct parahotplug_request *req;
> +
> +	req = kmalloc(sizeof(struct parahotplug_request), GFP_KERNEL|__GFP_NORETRY);
if you make it as:
	req = kmalloc(sizeof(*req), GFP_KERNEL|__GFP_NORETRY);
then this will not give you checkpatch warning of line over 80 char.

regards
sudip

>  	if (req = NULL)
>  		return NULL;
<snip>	
>  		return NULL;
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] staging: unisys: Remove allocation from declaration line

[Greg Kroah-Hartman]

On Tue, Feb 10, 2015 at 02:02:14PM +0100, Quentin Lambert wrote:
> This patch removes allocation from declaration line because
> people are known to gloss over declarations.

Again, who are these lazy people, and why are they reading kernel code?

Please, it's ok to allocate things in the declaration line, don't make
these kinds of changes.

greg k-h

Re: [PATCH 1/1] staging: unisys: Remove allocation from declaration line

[Greg Kroah-Hartman]

On Tue, Feb 10, 2015 at 02:02:14PM +0100, Quentin Lambert wrote:
> This patch removes allocation from declaration line because
> people are known to gloss over declarations.

Again, who are these lazy people, and why are they reading kernel code?

Please, it's ok to allocate things in the declaration line, don't make
these kinds of changes.

greg k-h

Re: [PATCH 1/1] staging: unisys: Remove allocation from declaration line

[Dan Carpenter]

On Wed, Feb 11, 2015 at 06:26:27AM +0800, Greg Kroah-Hartman wrote:
> On Tue, Feb 10, 2015 at 02:02:14PM +0100, Quentin Lambert wrote:
> > This patch removes allocation from declaration line because
> > people are known to gloss over declarations.
> 
> Again, who are these lazy people, and why are they reading kernel code?
> 

>From my work with smatch:
1) Probably 70-80% of inconsistent NULL checking is when done in the
   initializer.  I'm sending a patch for one of these today.
2) If there is an allocation in the initializer then it's more likely
   that the NULL check will be missing.
Initializers are a blind spot that people do not read.  It's not just
one maintainer, it's consistent across the board.

Also if you put an allocation in the initializer then it almost always
has to be mangled to fit in 80 characters and it looks ugly.  But after
these patches then all the allocations fit naturally.

Plus you have to have that blank line to separate the initialization
paragraph from the paragraph with the check for allocation failure.

Really, it is fairly uncommon to put an allocation in the initalizer.

regards,
dan carpenter

Re: [PATCH 1/1] staging: unisys: Remove allocation from declaration line

[Dan Carpenter]

On Wed, Feb 11, 2015 at 06:26:27AM +0800, Greg Kroah-Hartman wrote:
> On Tue, Feb 10, 2015 at 02:02:14PM +0100, Quentin Lambert wrote:
> > This patch removes allocation from declaration line because
> > people are known to gloss over declarations.
> 
> Again, who are these lazy people, and why are they reading kernel code?
> 

From my work with smatch:
1) Probably 70-80% of inconsistent NULL checking is when done in the
   initializer.  I'm sending a patch for one of these today.
2) If there is an allocation in the initializer then it's more likely
   that the NULL check will be missing.
Initializers are a blind spot that people do not read.  It's not just
one maintainer, it's consistent across the board.

Also if you put an allocation in the initializer then it almost always
has to be mangled to fit in 80 characters and it looks ugly.  But after
these patches then all the allocations fit naturally.

Plus you have to have that blank line to separate the initialization
paragraph from the paragraph with the check for allocation failure.

Really, it is fairly uncommon to put an allocation in the initalizer.

regards,
dan carpenter

Re: [PATCH 1/1] staging: unisys: Remove allocation from declaration line

[Quentin Lambert]

On 11/02/2015 11:23, Dan Carpenter wrote:
> On Wed, Feb 11, 2015 at 06:26:27AM +0800, Greg Kroah-Hartman wrote:
>> On Tue, Feb 10, 2015 at 02:02:14PM +0100, Quentin Lambert wrote:
>>> This patch removes allocation from declaration line because
>>> people are known to gloss over declarations.
>> Again, who are these lazy people, and why are they reading kernel code?
>>
>  From my work with smatch:
> 1) Probably 70-80% of inconsistent NULL checking is when done in the
>     initializer.  I'm sending a patch for one of these today.
> 2) If there is an allocation in the initializer then it's more likely
>     that the NULL check will be missing.
> Initializers are a blind spot that people do not read.  It's not just
> one maintainer, it's consistent across the board.
>
> Also if you put an allocation in the initializer then it almost always
> has to be mangled to fit in 80 characters and it looks ugly.  But after
> these patches then all the allocations fit naturally.
>
> Plus you have to have that blank line to separate the initialization
> paragraph from the paragraph with the check for allocation failure.
>
> Really, it is fairly uncommon to put an allocation in the initalizer.
>
> regards,
> dan carpenter
In the case this patch wasn't accepted what should I do with
this one: https://lkml.org/lkml/2015/2/10/182 ?

Do you want me to submit a non-dependent version?

regards,
Quentin

Re: [PATCH 1/1] staging: unisys: Remove allocation from declaration line

[Quentin Lambert]

On 11/02/2015 11:23, Dan Carpenter wrote:
> On Wed, Feb 11, 2015 at 06:26:27AM +0800, Greg Kroah-Hartman wrote:
>> On Tue, Feb 10, 2015 at 02:02:14PM +0100, Quentin Lambert wrote:
>>> This patch removes allocation from declaration line because
>>> people are known to gloss over declarations.
>> Again, who are these lazy people, and why are they reading kernel code?
>>
>  From my work with smatch:
> 1) Probably 70-80% of inconsistent NULL checking is when done in the
>     initializer.  I'm sending a patch for one of these today.
> 2) If there is an allocation in the initializer then it's more likely
>     that the NULL check will be missing.
> Initializers are a blind spot that people do not read.  It's not just
> one maintainer, it's consistent across the board.
>
> Also if you put an allocation in the initializer then it almost always
> has to be mangled to fit in 80 characters and it looks ugly.  But after
> these patches then all the allocations fit naturally.
>
> Plus you have to have that blank line to separate the initialization
> paragraph from the paragraph with the check for allocation failure.
>
> Really, it is fairly uncommon to put an allocation in the initalizer.
>
> regards,
> dan carpenter
In the case this patch wasn't accepted what should I do with
this one: https://lkml.org/lkml/2015/2/10/182 ?

Do you want me to submit a non-dependent version?

regards,
Quentin