All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5] staging: gasket: Create a free memory error path
@ 2018-10-31 21:37 Ioannis Valasakis
  2018-10-31 21:45 ` [Outreachy kernel] " Julia Lawall
  0 siblings, 1 reply; 5+ messages in thread
From: Ioannis Valasakis @ 2018-10-31 21:37 UTC (permalink / raw)
  To: outreachy-kernel; +Cc: gregkh, alexander.levin

Create an error handling path for memory allocation. It avoids repeating
the same assignments and returns the respective ENOMEM.

Signed-off-by: Ioannis Valasakis <code@wizofe.uk>
---
Changes in v5:
	- Typo on the error handling label

drivers/staging/gasket/gasket_interrupt.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/gasket/gasket_interrupt.c b/drivers/staging/gasket/gasket_interrupt.c
index 49d47afad64f..1a3e8b17e714 100644
--- a/drivers/staging/gasket/gasket_interrupt.c
+++ b/drivers/staging/gasket/gasket_interrupt.c
@@ -340,17 +340,14 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev)
 					       sizeof(struct eventfd_ctx *),
 					       GFP_KERNEL);
 	if (!interrupt_data->eventfd_ctxs) {
-		kfree(interrupt_data);
-		return -ENOMEM;
+		goto err_free_ctxs;
 	}
 
 	interrupt_data->interrupt_counts = kcalloc(driver_desc->num_interrupts,
 						   sizeof(ulong),
 						   GFP_KERNEL);
 	if (!interrupt_data->interrupt_counts) {
-		kfree(interrupt_data->eventfd_ctxs);
-		kfree(interrupt_data);
-		return -ENOMEM;
+		goto err_free_counts;
 	}
 
 	switch (interrupt_data->type) {
@@ -379,6 +376,12 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev)
 				    interrupt_sysfs_attrs);
 
 	return 0;
+
+err_free_counts:
+	kfree(interrupt_data->eventfd_ctxs);
+err_free_ctxs:
+	kfree(interrupt_data);
+	return -ENOMEM;
 }
 
 static void
-- 
2.19.1




^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [Outreachy kernel] [PATCH v5] staging: gasket: Create a free memory error path
  2018-10-31 21:37 [PATCH v5] staging: gasket: Create a free memory error path Ioannis Valasakis
@ 2018-10-31 21:45 ` Julia Lawall
  2018-10-31 22:02   ` Ioannis Valasakis
  0 siblings, 1 reply; 5+ messages in thread
From: Julia Lawall @ 2018-10-31 21:45 UTC (permalink / raw)
  To: Ioannis Valasakis; +Cc: outreachy-kernel, gregkh, alexander.levin



On Wed, 31 Oct 2018, Ioannis Valasakis wrote:

> Create an error handling path for memory allocation. It avoids repeating
> the same assignments and returns the respective ENOMEM.
>
> Signed-off-by: Ioannis Valasakis <code@wizofe.uk>
> ---
> Changes in v5:
> 	- Typo on the error handling label

Compilation should have found this error.

> drivers/staging/gasket/gasket_interrupt.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/gasket/gasket_interrupt.c b/drivers/staging/gasket/gasket_interrupt.c
> index 49d47afad64f..1a3e8b17e714 100644
> --- a/drivers/staging/gasket/gasket_interrupt.c
> +++ b/drivers/staging/gasket/gasket_interrupt.c
> @@ -340,17 +340,14 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev)
>  					       sizeof(struct eventfd_ctx *),
>  					       GFP_KERNEL);
>  	if (!interrupt_data->eventfd_ctxs) {
> -		kfree(interrupt_data);
> -		return -ENOMEM;
> +		goto err_free_ctxs;

Your labels describe where you are coming from.  Dan suggests to make the
labels descibe what will happen when you get there.  Here you aren't going
to be freeing ctxs.  That is the allocation that failed.

julia

>  	}
>
>  	interrupt_data->interrupt_counts = kcalloc(driver_desc->num_interrupts,
>  						   sizeof(ulong),
>  						   GFP_KERNEL);
>  	if (!interrupt_data->interrupt_counts) {
> -		kfree(interrupt_data->eventfd_ctxs);
> -		kfree(interrupt_data);
> -		return -ENOMEM;
> +		goto err_free_counts;
>  	}
>
>  	switch (interrupt_data->type) {
> @@ -379,6 +376,12 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev)
>  				    interrupt_sysfs_attrs);
>
>  	return 0;
> +
> +err_free_counts:
> +	kfree(interrupt_data->eventfd_ctxs);
> +err_free_ctxs:
> +	kfree(interrupt_data);
> +	return -ENOMEM;
>  }
>
>  static void
> --
> 2.19.1
>
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20181031213742.GA18868%40kvasir.local.
> For more options, visit https://groups.google.com/d/optout.
>


^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [Outreachy kernel] [PATCH v5] staging: gasket: Create a free memory error path
  2018-10-31 21:45 ` [Outreachy kernel] " Julia Lawall
@ 2018-10-31 22:02   ` Ioannis Valasakis
  2018-11-01  4:36     ` Himanshu Jha
  2018-11-01  6:27     ` Julia Lawall
  0 siblings, 2 replies; 5+ messages in thread
From: Ioannis Valasakis @ 2018-10-31 22:02 UTC (permalink / raw)
  To: Julia Lawall; +Cc: outreachy-kernel, gregkh, alexander.levin

On Wed, Oct 31, 2018 at 10:45:33PM +0100, Julia Lawall wrote:
> 
> 
> On Wed, 31 Oct 2018, Ioannis Valasakis wrote:
> 
> > Create an error handling path for memory allocation. It avoids repeating
> > the same assignments and returns the respective ENOMEM.
> >
> > Signed-off-by: Ioannis Valasakis <code@wizofe.uk>
> > ---
> > Changes in v5:
> > 	- Typo on the error handling label
> 
> Compilation should have found this error.
It would indeed but it still's going on since an hour now so I wanted to
get your feedback meanwhile and understand my mistakes! Me rushing isn't
good but after it compiles with all the new changes I should have a very
fast turnaround :) 

> 
> > drivers/staging/gasket/gasket_interrupt.c | 13 ++++++++-----
> >  1 file changed, 8 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/staging/gasket/gasket_interrupt.c b/drivers/staging/gasket/gasket_interrupt.c
> > index 49d47afad64f..1a3e8b17e714 100644
> > --- a/drivers/staging/gasket/gasket_interrupt.c
> > +++ b/drivers/staging/gasket/gasket_interrupt.c
> > @@ -340,17 +340,14 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev)
> >  					       sizeof(struct eventfd_ctx *),
> >  					       GFP_KERNEL);
> >  	if (!interrupt_data->eventfd_ctxs) {
> > -		kfree(interrupt_data);
> > -		return -ENOMEM;
> > +		goto err_free_ctxs;
> 
> Your labels describe where you are coming from.  Dan suggests to make the
> labels descibe what will happen when you get there.  Here you aren't going
> to be freeing ctxs.  That is the allocation that failed.
> 
In that case is err_free_eventfd and err_free_data a good naming scheme? 

ta
Ioannis

> julia
> 
> >  	}
> >
> >  	interrupt_data->interrupt_counts = kcalloc(driver_desc->num_interrupts,
> >  						   sizeof(ulong),
> >  						   GFP_KERNEL);
> >  	if (!interrupt_data->interrupt_counts) {
> > -		kfree(interrupt_data->eventfd_ctxs);
> > -		kfree(interrupt_data);
> > -		return -ENOMEM;
> > +		goto err_free_counts;
> >  	}
> >
> >  	switch (interrupt_data->type) {
> > @@ -379,6 +376,12 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev)
> >  				    interrupt_sysfs_attrs);
> >
> >  	return 0;
> > +
> > +err_free_counts:
> > +	kfree(interrupt_data->eventfd_ctxs);
> > +err_free_ctxs:
> > +	kfree(interrupt_data);
> > +	return -ENOMEM;
> >  }
> >
> >  static void
> > --
> > 2.19.1
> >
> >
> > --
> > You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> > To post to this group, send email to outreachy-kernel@googlegroups.com.
> > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20181031213742.GA18868%40kvasir.local.
> > For more options, visit https://groups.google.com/d/optout.
> >



^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [Outreachy kernel] [PATCH v5] staging: gasket: Create a free memory error path
  2018-10-31 22:02   ` Ioannis Valasakis
@ 2018-11-01  4:36     ` Himanshu Jha
  2018-11-01  6:27     ` Julia Lawall
  1 sibling, 0 replies; 5+ messages in thread
From: Himanshu Jha @ 2018-11-01  4:36 UTC (permalink / raw)
  To: Ioannis Valasakis; +Cc: Julia Lawall, outreachy-kernel, gregkh, alexander.levin

On Wed, Oct 31, 2018 at 10:02:43PM +0000, Ioannis Valasakis wrote:
> On Wed, Oct 31, 2018 at 10:45:33PM +0100, Julia Lawall wrote:
> > 
> > 
> > On Wed, 31 Oct 2018, Ioannis Valasakis wrote:
> > 
> > > Create an error handling path for memory allocation. It avoids repeating
> > > the same assignments and returns the respective ENOMEM.
> > >
> > > Signed-off-by: Ioannis Valasakis <code@wizofe.uk>
> > > ---
> > > Changes in v5:
> > > 	- Typo on the error handling label
> > 
> > Compilation should have found this error.
> It would indeed but it still's going on since an hour now so I wanted to
> get your feedback meanwhile and understand my mistakes! Me rushing isn't
> good but after it compiles with all the new changes I should have a very
> fast turnaround :) 

Wait..what ?

Are you compiling the whole kernel for this and past checkpatch changes ?

If you are, then please look at partial building techniques such as:

# make drivers/staging/gasket/gasket_interrupt.o

or

# make M=drivers/staging/gasket/

and you could use -j(jobs) to speed up compilation.

Read Linux kernel in a Nutshell for more detailed info.


-- 
Himanshu Jha
Undergraduate Student
Department of Electronics & Communication
Guru Tegh Bahadur Institute of Technology


^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [Outreachy kernel] [PATCH v5] staging: gasket: Create a free memory error path
  2018-10-31 22:02   ` Ioannis Valasakis
  2018-11-01  4:36     ` Himanshu Jha
@ 2018-11-01  6:27     ` Julia Lawall
  1 sibling, 0 replies; 5+ messages in thread
From: Julia Lawall @ 2018-11-01  6:27 UTC (permalink / raw)
  To: Ioannis Valasakis; +Cc: outreachy-kernel, gregkh, alexander.levin



On Wed, 31 Oct 2018, Ioannis Valasakis wrote:

> On Wed, Oct 31, 2018 at 10:45:33PM +0100, Julia Lawall wrote:
> >
> >
> > On Wed, 31 Oct 2018, Ioannis Valasakis wrote:
> >
> > > Create an error handling path for memory allocation. It avoids repeating
> > > the same assignments and returns the respective ENOMEM.
> > >
> > > Signed-off-by: Ioannis Valasakis <code@wizofe.uk>
> > > ---
> > > Changes in v5:
> > > 	- Typo on the error handling label
> >
> > Compilation should have found this error.
> It would indeed but it still's going on since an hour now so I wanted to
> get your feedback meanwhile and understand my mistakes! Me rushing isn't
> good but after it compiles with all the new changes I should have a very
> fast turnaround :)

You can often compile just the file you are working on by saying make
foo.o.  You can reread the Compiling only part of the kernel section of
the tutorial.

> > > drivers/staging/gasket/gasket_interrupt.c | 13 ++++++++-----
> > >  1 file changed, 8 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/staging/gasket/gasket_interrupt.c b/drivers/staging/gasket/gasket_interrupt.c
> > > index 49d47afad64f..1a3e8b17e714 100644
> > > --- a/drivers/staging/gasket/gasket_interrupt.c
> > > +++ b/drivers/staging/gasket/gasket_interrupt.c
> > > @@ -340,17 +340,14 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev)
> > >  					       sizeof(struct eventfd_ctx *),
> > >  					       GFP_KERNEL);
> > >  	if (!interrupt_data->eventfd_ctxs) {
> > > -		kfree(interrupt_data);
> > > -		return -ENOMEM;
> > > +		goto err_free_ctxs;
> >
> > Your labels describe where you are coming from.  Dan suggests to make the
> > labels descibe what will happen when you get there.  Here you aren't going
> > to be freeing ctxs.  That is the allocation that failed.
> >
> In that case is err_free_eventfd and err_free_data a good naming scheme?

Maybe err_free_ctxs.

julia

>
> ta
> Ioannis
>
> > julia
> >
> > >  	}
> > >
> > >  	interrupt_data->interrupt_counts = kcalloc(driver_desc->num_interrupts,
> > >  						   sizeof(ulong),
> > >  						   GFP_KERNEL);
> > >  	if (!interrupt_data->interrupt_counts) {
> > > -		kfree(interrupt_data->eventfd_ctxs);
> > > -		kfree(interrupt_data);
> > > -		return -ENOMEM;
> > > +		goto err_free_counts;
> > >  	}
> > >
> > >  	switch (interrupt_data->type) {
> > > @@ -379,6 +376,12 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev)
> > >  				    interrupt_sysfs_attrs);
> > >
> > >  	return 0;
> > > +
> > > +err_free_counts:
> > > +	kfree(interrupt_data->eventfd_ctxs);
> > > +err_free_ctxs:
> > > +	kfree(interrupt_data);
> > > +	return -ENOMEM;
> > >  }
> > >
> > >  static void
> > > --
> > > 2.19.1
> > >
> > >
> > > --
> > > You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> > > To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> > > To post to this group, send email to outreachy-kernel@googlegroups.com.
> > > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20181031213742.GA18868%40kvasir.local.
> > > For more options, visit https://groups.google.com/d/optout.
> > >
>
>


^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-11-01  6:27 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-31 21:37 [PATCH v5] staging: gasket: Create a free memory error path Ioannis Valasakis
2018-10-31 21:45 ` [Outreachy kernel] " Julia Lawall
2018-10-31 22:02   ` Ioannis Valasakis
2018-11-01  4:36     ` Himanshu Jha
2018-11-01  6:27     ` Julia Lawall

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.