Re: [Outreachy kernel] [PATCH v5] staging: gasket: Create a free memory error path

[Ioannis Valasakis <code@wizofe.uk>]

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.
> >