Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1512179 > unrolled thread

[PATCH] cxl: Fix memory allocation failure test

Started byChristophe JAILLET <christophe.jaillet@wanadoo.fr>
First post2016-10-30 21:00 +0100
Last post2016-11-02 17:50 +0100
Articles 4 — 4 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH] cxl: Fix memory allocation failure test Christophe JAILLET <christophe.jaillet@wanadoo.fr> - 2016-10-30 21:00 +0100
    Re: [PATCH] cxl: Fix memory allocation failure test walter harms <wharms@bfs.de> - 2016-10-30 21:20 +0100
    Re: [PATCH] cxl: Fix memory allocation failure test Andrew Donnellan <andrew.donnellan@au1.ibm.com> - 2016-10-31 07:20 +0100
    Re: [PATCH] cxl: Fix memory allocation failure test Frederic Barrat <fbarrat@linux.vnet.ibm.com> - 2016-11-02 17:50 +0100

#1512179 — [PATCH] cxl: Fix memory allocation failure test

FromChristophe JAILLET <christophe.jaillet@wanadoo.fr>
Date2016-10-30 21:00 +0100
Subject[PATCH] cxl: Fix memory allocation failure test
Message-ID<sy4qR-wo-1@gated-at.bofh.it>
'cxl_context_alloc()' does not return an error pointer. It is just a
shortcut for a call to 'kzalloc' with 'sizeof(struct cxl_context)' as the
size parameter.

So its return value should be compared with NULL.
While fixing it, simplify a bit the code.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
un-compiled because I don't have the required  cross build environment.
---
 drivers/misc/cxl/api.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/misc/cxl/api.c b/drivers/misc/cxl/api.c
index 2e5233b60971..2b88ad8a2a89 100644
--- a/drivers/misc/cxl/api.c
+++ b/drivers/misc/cxl/api.c
@@ -30,10 +30,8 @@ struct cxl_context *cxl_dev_context_init(struct pci_dev *dev)
 		return ERR_CAST(afu);
 
 	ctx = cxl_context_alloc();
-	if (IS_ERR(ctx)) {
-		rc = PTR_ERR(ctx);
-		goto err_dev;
-	}
+	if (!ctx)
+		return ERR_PTR(-ENOMEM);
 
 	ctx->kernelapi = true;
 
@@ -61,7 +59,6 @@ struct cxl_context *cxl_dev_context_init(struct pci_dev *dev)
 	kfree(mapping);
 err_ctx:
 	kfree(ctx);
-err_dev:
 	return ERR_PTR(rc);
 }
 EXPORT_SYMBOL_GPL(cxl_dev_context_init);
-- 
2.9.3

[toc] | [next] | [standalone]


#1512181

Fromwalter harms <wharms@bfs.de>
Date2016-10-30 21:20 +0100
Message-ID<sy4Kd-Wo-5@gated-at.bofh.it>
In reply to#1512179

Am 30.10.2016 20:35, schrieb Christophe JAILLET:
> 'cxl_context_alloc()' does not return an error pointer. It is just a
> shortcut for a call to 'kzalloc' with 'sizeof(struct cxl_context)' as the
> size parameter.
> 
> So its return value should be compared with NULL.
> While fixing it, simplify a bit the code.
> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> un-compiled because I don't have the required  cross build environment.
> ---
>  drivers/misc/cxl/api.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/misc/cxl/api.c b/drivers/misc/cxl/api.c
> index 2e5233b60971..2b88ad8a2a89 100644
> --- a/drivers/misc/cxl/api.c
> +++ b/drivers/misc/cxl/api.c
> @@ -30,10 +30,8 @@ struct cxl_context *cxl_dev_context_init(struct pci_dev *dev)
>  		return ERR_CAST(afu);
>  
>  	ctx = cxl_context_alloc();
> -	if (IS_ERR(ctx)) {
> -		rc = PTR_ERR(ctx);
> -		goto err_dev;
> -	}
> +	if (!ctx)
> +		return ERR_PTR(-ENOMEM);
>  

So far i see it is only used 2 times,
To avoid such problems it should be replaced with
    kzalloc(sizeof(struct cxl_context), GFP_KERNEL); (from context.c)

or even better:
	ctx = kzalloc(*ctx, GFP_KERNEL);

This way the error had been spotted much more early.

just my 2 cents,
re,
 wh

>  	ctx->kernelapi = true;
>  
> @@ -61,7 +59,6 @@ struct cxl_context *cxl_dev_context_init(struct pci_dev *dev)
>  	kfree(mapping);
>  err_ctx:
>  	kfree(ctx);
> -err_dev:
>  	return ERR_PTR(rc);
>  }
>  EXPORT_SYMBOL_GPL(cxl_dev_context_init);

[toc] | [prev] | [next] | [standalone]


#1512364

FromAndrew Donnellan <andrew.donnellan@au1.ibm.com>
Date2016-10-31 07:20 +0100
Message-ID<sye6R-74m-7@gated-at.bofh.it>
In reply to#1512179
On 31/10/16 06:35, Christophe JAILLET wrote:
> 'cxl_context_alloc()' does not return an error pointer. It is just a
> shortcut for a call to 'kzalloc' with 'sizeof(struct cxl_context)' as the
> size parameter.
>
> So its return value should be compared with NULL.
> While fixing it, simplify a bit the code.
>
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>

Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>

> ---
> un-compiled because I don't have the required  cross build environment.

Compiles on my system!

-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com  IBM Australia Limited

[toc] | [prev] | [next] | [standalone]


#1513973

FromFrederic Barrat <fbarrat@linux.vnet.ibm.com>
Date2016-11-02 17:50 +0100
Message-ID<sz6TE-jL-29@gated-at.bofh.it>
In reply to#1512179

Le 30/10/2016 à 20:35, Christophe JAILLET a écrit :
> 'cxl_context_alloc()' does not return an error pointer. It is just a
> shortcut for a call to 'kzalloc' with 'sizeof(struct cxl_context)' as the
> size parameter.
>
> So its return value should be compared with NULL.
> While fixing it, simplify a bit the code.
>
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---


Acked-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web