* [PATCH v2] staging: gasket Create a memory allocation path @ 2018-10-31 16:47 Ioannis Valasakis 2018-10-31 16:54 ` [Outreachy kernel] " Sasha Levin 2018-10-31 20:40 ` Julia Lawall 0 siblings, 2 replies; 8+ messages in thread From: Ioannis Valasakis @ 2018-10-31 16:47 UTC (permalink / raw) To: outreachy-kernel; +Cc: gregkh, alexander.levin Fix multiple error allocation path by using re-direction. Signed-off-by: Ioannis Valasakis <code@wizofe.uk> --- Changes in v2: - Specific tags for memory allocation - Free the memory in the right way as for kfre drivers/staging/gasket/gasket_interrupt.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/drivers/staging/gasket/gasket_interrupt.c b/drivers/staging/gasket/gasket_interrupt.c index cdfb72af1c52..23232ea8f4d9 100644 --- a/drivers/staging/gasket/gasket_interrupt.c +++ b/drivers/staging/gasket/gasket_interrupt.c @@ -348,21 +348,21 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev, const char *name, sizeof(struct msix_entry), GFP_KERNEL); if (!interrupt_data->msix_entries) { - goto err_alloc; + goto err_free_msix; } interrupt_data->eventfd_ctxs = kcalloc(num_interrupts, sizeof(struct eventfd_ctx *), GFP_KERNEL); if (!interrupt_data->eventfd_ctxs) { - goto err_alloc; + goto err_free_eventfd; } interrupt_data->interrupt_counts = kcalloc(num_interrupts, sizeof(ulong), GFP_KERNEL); if (!interrupt_data->interrupt_counts) { - goto err_alloc; + goto err_free_data; } switch (interrupt_data->type) { @@ -397,10 +397,12 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev, const char *name, return 0; -err_alloc: - kfree(interrupt_data); - kfree(interrupt_data->msix_entries); +err_free_msix: kfree(interrupt_data->eventfd_ctxs); +err_free_eventfd: + kfree(interrupt_data->msix_entries); +err_free_data: + kfree(interrupt_data); return -ENOMEM; } -- 2.19.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Outreachy kernel] [PATCH v2] staging: gasket Create a memory allocation path 2018-10-31 16:47 [PATCH v2] staging: gasket Create a memory allocation path Ioannis Valasakis @ 2018-10-31 16:54 ` Sasha Levin 2018-10-31 20:50 ` Ioannis Valasakis 2018-10-31 20:40 ` Julia Lawall 1 sibling, 1 reply; 8+ messages in thread From: Sasha Levin @ 2018-10-31 16:54 UTC (permalink / raw) To: Ioannis Valasakis; +Cc: outreachy-kernel, gregkh, alexander.levin On Wed, Oct 31, 2018 at 04:47:54PM +0000, Ioannis Valasakis wrote: >Fix multiple error allocation path by using re-direction. > >Signed-off-by: Ioannis Valasakis <code@wizofe.uk> >--- >Changes in v2: > - Specific tags for memory allocation > - Free the memory in the right way as for kfre > >drivers/staging/gasket/gasket_interrupt.c | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) > >diff --git a/drivers/staging/gasket/gasket_interrupt.c b/drivers/staging/gasket/gasket_interrupt.c >index cdfb72af1c52..23232ea8f4d9 100644 >--- a/drivers/staging/gasket/gasket_interrupt.c >+++ b/drivers/staging/gasket/gasket_interrupt.c >@@ -348,21 +348,21 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev, const char *name, > sizeof(struct msix_entry), > GFP_KERNEL); > if (!interrupt_data->msix_entries) { >- goto err_alloc; >+ goto err_free_msix; > } > > interrupt_data->eventfd_ctxs = kcalloc(num_interrupts, > sizeof(struct eventfd_ctx *), > GFP_KERNEL); > if (!interrupt_data->eventfd_ctxs) { >- goto err_alloc; >+ goto err_free_eventfd; > } > > interrupt_data->interrupt_counts = kcalloc(num_interrupts, > sizeof(ulong), > GFP_KERNEL); > if (!interrupt_data->interrupt_counts) { >- goto err_alloc; >+ goto err_free_data; > } > > switch (interrupt_data->type) { >@@ -397,10 +397,12 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev, const char *name, > > return 0; > >-err_alloc: >- kfree(interrupt_data); >- kfree(interrupt_data->msix_entries); >+err_free_msix: > kfree(interrupt_data->eventfd_ctxs); >+err_free_eventfd: >+ kfree(interrupt_data->msix_entries); >+err_free_data: >+ kfree(interrupt_data); > return -ENOMEM; > } With this code, what will happen if, for example, the last allocation (interrupt_data->interrupt_counts) fails? -- Thanks, Sasha ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Outreachy kernel] [PATCH v2] staging: gasket Create a memory allocation path 2018-10-31 16:54 ` [Outreachy kernel] " Sasha Levin @ 2018-10-31 20:50 ` Ioannis Valasakis 2018-10-31 20:57 ` Julia Lawall 0 siblings, 1 reply; 8+ messages in thread From: Ioannis Valasakis @ 2018-10-31 20:50 UTC (permalink / raw) To: Sasha Levin; +Cc: outreachy-kernel, gregkh, alexander.levin On Wed, Oct 31, 2018 at 12:54:39PM -0400, Sasha Levin wrote: > On Wed, Oct 31, 2018 at 04:47:54PM +0000, Ioannis Valasakis wrote: > > Fix multiple error allocation path by using re-direction. > > > > Signed-off-by: Ioannis Valasakis <code@wizofe.uk> > > --- > > Changes in v2: > > - Specific tags for memory allocation > > - Free the memory in the right way as for kfre > > > > drivers/staging/gasket/gasket_interrupt.c | 14 ++++++++------ > > 1 file changed, 8 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/staging/gasket/gasket_interrupt.c b/drivers/staging/gasket/gasket_interrupt.c > > index cdfb72af1c52..23232ea8f4d9 100644 > > --- a/drivers/staging/gasket/gasket_interrupt.c > > +++ b/drivers/staging/gasket/gasket_interrupt.c > > @@ -348,21 +348,21 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev, const char *name, > > sizeof(struct msix_entry), > > GFP_KERNEL); > > if (!interrupt_data->msix_entries) { > > - goto err_alloc; > > + goto err_free_msix; > > } > > > > interrupt_data->eventfd_ctxs = kcalloc(num_interrupts, > > sizeof(struct eventfd_ctx *), > > GFP_KERNEL); > > if (!interrupt_data->eventfd_ctxs) { > > - goto err_alloc; > > + goto err_free_eventfd; > > } > > > > interrupt_data->interrupt_counts = kcalloc(num_interrupts, > > sizeof(ulong), > > GFP_KERNEL); > > if (!interrupt_data->interrupt_counts) { > > - goto err_alloc; > > + goto err_free_data; > > } > > > > switch (interrupt_data->type) { > > @@ -397,10 +397,12 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev, const char *name, > > > > return 0; > > > > -err_alloc: > > - kfree(interrupt_data); > > - kfree(interrupt_data->msix_entries); > > +err_free_msix: > > kfree(interrupt_data->eventfd_ctxs); > > +err_free_eventfd: > > + kfree(interrupt_data->msix_entries); > > +err_free_data: > > + kfree(interrupt_data); > > return -ENOMEM; > > } > > With this code, what will happen if, for example, the last > allocation (interrupt_data->interrupt_counts) fails? It took my some time but I think I saw it; the mistake is that if interrupt_data->interrupt_counts fails it should 'goto err_free_msix'. Same if interrupt_data->msix_entries fails it should 'goto err_free_data'. Is that right? (I got very ambitious with the labels and I mixed it up ;) Any way to make the goto labels make sense?) > > -- > Thanks, > Sasha Best, Ioannis ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Outreachy kernel] [PATCH v2] staging: gasket Create a memory allocation path 2018-10-31 20:50 ` Ioannis Valasakis @ 2018-10-31 20:57 ` Julia Lawall 2018-10-31 21:00 ` Ioannis Valasakis 0 siblings, 1 reply; 8+ messages in thread From: Julia Lawall @ 2018-10-31 20:57 UTC (permalink / raw) To: Ioannis Valasakis; +Cc: Sasha Levin, outreachy-kernel, gregkh, alexander.levin On Wed, 31 Oct 2018, Ioannis Valasakis wrote: > On Wed, Oct 31, 2018 at 12:54:39PM -0400, Sasha Levin wrote: > > On Wed, Oct 31, 2018 at 04:47:54PM +0000, Ioannis Valasakis wrote: > > > Fix multiple error allocation path by using re-direction. > > > > > > Signed-off-by: Ioannis Valasakis <code@wizofe.uk> > > > --- > > > Changes in v2: > > > - Specific tags for memory allocation > > > - Free the memory in the right way as for kfre > > > > > > drivers/staging/gasket/gasket_interrupt.c | 14 ++++++++------ > > > 1 file changed, 8 insertions(+), 6 deletions(-) > > > > > > diff --git a/drivers/staging/gasket/gasket_interrupt.c b/drivers/staging/gasket/gasket_interrupt.c > > > index cdfb72af1c52..23232ea8f4d9 100644 > > > --- a/drivers/staging/gasket/gasket_interrupt.c > > > +++ b/drivers/staging/gasket/gasket_interrupt.c > > > @@ -348,21 +348,21 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev, const char *name, > > > sizeof(struct msix_entry), > > > GFP_KERNEL); > > > if (!interrupt_data->msix_entries) { > > > - goto err_alloc; > > > + goto err_free_msix; > > > } > > > > > > interrupt_data->eventfd_ctxs = kcalloc(num_interrupts, > > > sizeof(struct eventfd_ctx *), > > > GFP_KERNEL); > > > if (!interrupt_data->eventfd_ctxs) { > > > - goto err_alloc; > > > + goto err_free_eventfd; > > > } > > > > > > interrupt_data->interrupt_counts = kcalloc(num_interrupts, > > > sizeof(ulong), > > > GFP_KERNEL); > > > if (!interrupt_data->interrupt_counts) { > > > - goto err_alloc; > > > + goto err_free_data; > > > } > > > > > > switch (interrupt_data->type) { > > > @@ -397,10 +397,12 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev, const char *name, > > > > > > return 0; > > > > > > -err_alloc: > > > - kfree(interrupt_data); > > > - kfree(interrupt_data->msix_entries); > > > +err_free_msix: > > > kfree(interrupt_data->eventfd_ctxs); > > > +err_free_eventfd: > > > + kfree(interrupt_data->msix_entries); > > > +err_free_data: > > > + kfree(interrupt_data); > > > return -ENOMEM; > > > } > > > > With this code, what will happen if, for example, the last > > allocation (interrupt_data->interrupt_counts) fails? > It took my some time but I think I saw it; the mistake is that > if interrupt_data->interrupt_counts fails it should 'goto > err_free_msix'. > > Same if interrupt_data->msix_entries fails it should 'goto > err_free_data'. > > Is that right? > > (I got very ambitious with the labels and I mixed it up ;) Any way to > make the goto labels make sense?) I don't think that the choice of labels is a problem. Basically, you should normally release things in the opposite order of the one in which they were allocated. At the beginning of your function, you can just do a return, because nothing has been allocated yet. Then when you have successfully allocated something, if you fail afterwards you should go to the very end of the function, because you have only one thing to free. When you have successfully allocated something else, if you fail afterwards, you should jump not quite to the end, so that you free both things that have been allocated, etc. julia ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Outreachy kernel] [PATCH v2] staging: gasket Create a memory allocation path 2018-10-31 20:57 ` Julia Lawall @ 2018-10-31 21:00 ` Ioannis Valasakis 2018-10-31 21:04 ` Julia Lawall 0 siblings, 1 reply; 8+ messages in thread From: Ioannis Valasakis @ 2018-10-31 21:00 UTC (permalink / raw) To: Julia Lawall; +Cc: Sasha Levin, outreachy-kernel, gregkh, alexander.levin On Wed, Oct 31, 2018 at 09:57:58PM +0100, Julia Lawall wrote: > > > On Wed, 31 Oct 2018, Ioannis Valasakis wrote: > > > On Wed, Oct 31, 2018 at 12:54:39PM -0400, Sasha Levin wrote: > > > On Wed, Oct 31, 2018 at 04:47:54PM +0000, Ioannis Valasakis wrote: > > > > Fix multiple error allocation path by using re-direction. > > > > > > > > Signed-off-by: Ioannis Valasakis <code@wizofe.uk> > > > > --- > > > > Changes in v2: > > > > - Specific tags for memory allocation > > > > - Free the memory in the right way as for kfre > > > > > > > > drivers/staging/gasket/gasket_interrupt.c | 14 ++++++++------ > > > > 1 file changed, 8 insertions(+), 6 deletions(-) > > > > > > > > diff --git a/drivers/staging/gasket/gasket_interrupt.c b/drivers/staging/gasket/gasket_interrupt.c > > > > index cdfb72af1c52..23232ea8f4d9 100644 > > > > --- a/drivers/staging/gasket/gasket_interrupt.c > > > > +++ b/drivers/staging/gasket/gasket_interrupt.c > > > > @@ -348,21 +348,21 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev, const char *name, > > > > sizeof(struct msix_entry), > > > > GFP_KERNEL); > > > > if (!interrupt_data->msix_entries) { > > > > - goto err_alloc; > > > > + goto err_free_msix; > > > > } > > > > > > > > interrupt_data->eventfd_ctxs = kcalloc(num_interrupts, > > > > sizeof(struct eventfd_ctx *), > > > > GFP_KERNEL); > > > > if (!interrupt_data->eventfd_ctxs) { > > > > - goto err_alloc; > > > > + goto err_free_eventfd; > > > > } > > > > > > > > interrupt_data->interrupt_counts = kcalloc(num_interrupts, > > > > sizeof(ulong), > > > > GFP_KERNEL); > > > > if (!interrupt_data->interrupt_counts) { > > > > - goto err_alloc; > > > > + goto err_free_data; > > > > } > > > > > > > > switch (interrupt_data->type) { > > > > @@ -397,10 +397,12 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev, const char *name, > > > > > > > > return 0; > > > > > > > > -err_alloc: > > > > - kfree(interrupt_data); > > > > - kfree(interrupt_data->msix_entries); > > > > +err_free_msix: > > > > kfree(interrupt_data->eventfd_ctxs); > > > > +err_free_eventfd: > > > > + kfree(interrupt_data->msix_entries); > > > > +err_free_data: > > > > + kfree(interrupt_data); > > > > return -ENOMEM; > > > > } > > > > > > With this code, what will happen if, for example, the last > > > allocation (interrupt_data->interrupt_counts) fails? > > It took my some time but I think I saw it; the mistake is that > > if interrupt_data->interrupt_counts fails it should 'goto > > err_free_msix'. > > > > Same if interrupt_data->msix_entries fails it should 'goto > > err_free_data'. > > > > Is that right? > > > > (I got very ambitious with the labels and I mixed it up ;) Any way to > > make the goto labels make sense?) > > I don't think that the choice of labels is a problem. Basically, you > should normally release things in the opposite order of the one in which > they were allocated. At the beginning of your function, you can just do a > return, because nothing has been allocated yet. Then when you have > successfully allocated something, if you fail afterwards you should go to > the very end of the function, because you have only one thing to free. > When you have successfully allocated something else, if you fail > afterwards, you should jump not quite to the end, so that you free both > things that have been allocated, etc. Indeed that is what I meant. Badly phrased but I released things the wrong way around. I send a v3 of the patch (although as I just noticed your latest comment, if that makes sense I am going to rebase against greg's staging and resend it). Thank you Ioannis > > julia ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Outreachy kernel] [PATCH v2] staging: gasket Create a memory allocation path 2018-10-31 21:00 ` Ioannis Valasakis @ 2018-10-31 21:04 ` Julia Lawall 0 siblings, 0 replies; 8+ messages in thread From: Julia Lawall @ 2018-10-31 21:04 UTC (permalink / raw) To: Ioannis Valasakis; +Cc: Sasha Levin, outreachy-kernel, gregkh, alexander.levin On Wed, 31 Oct 2018, Ioannis Valasakis wrote: > On Wed, Oct 31, 2018 at 09:57:58PM +0100, Julia Lawall wrote: > > > > > > On Wed, 31 Oct 2018, Ioannis Valasakis wrote: > > > > > On Wed, Oct 31, 2018 at 12:54:39PM -0400, Sasha Levin wrote: > > > > On Wed, Oct 31, 2018 at 04:47:54PM +0000, Ioannis Valasakis wrote: > > > > > Fix multiple error allocation path by using re-direction. > > > > > > > > > > Signed-off-by: Ioannis Valasakis <code@wizofe.uk> > > > > > --- > > > > > Changes in v2: > > > > > - Specific tags for memory allocation > > > > > - Free the memory in the right way as for kfre > > > > > > > > > > drivers/staging/gasket/gasket_interrupt.c | 14 ++++++++------ > > > > > 1 file changed, 8 insertions(+), 6 deletions(-) > > > > > > > > > > diff --git a/drivers/staging/gasket/gasket_interrupt.c b/drivers/staging/gasket/gasket_interrupt.c > > > > > index cdfb72af1c52..23232ea8f4d9 100644 > > > > > --- a/drivers/staging/gasket/gasket_interrupt.c > > > > > +++ b/drivers/staging/gasket/gasket_interrupt.c > > > > > @@ -348,21 +348,21 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev, const char *name, > > > > > sizeof(struct msix_entry), > > > > > GFP_KERNEL); > > > > > if (!interrupt_data->msix_entries) { > > > > > - goto err_alloc; > > > > > + goto err_free_msix; > > > > > } > > > > > > > > > > interrupt_data->eventfd_ctxs = kcalloc(num_interrupts, > > > > > sizeof(struct eventfd_ctx *), > > > > > GFP_KERNEL); > > > > > if (!interrupt_data->eventfd_ctxs) { > > > > > - goto err_alloc; > > > > > + goto err_free_eventfd; > > > > > } > > > > > > > > > > interrupt_data->interrupt_counts = kcalloc(num_interrupts, > > > > > sizeof(ulong), > > > > > GFP_KERNEL); > > > > > if (!interrupt_data->interrupt_counts) { > > > > > - goto err_alloc; > > > > > + goto err_free_data; > > > > > } > > > > > > > > > > switch (interrupt_data->type) { > > > > > @@ -397,10 +397,12 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev, const char *name, > > > > > > > > > > return 0; > > > > > > > > > > -err_alloc: > > > > > - kfree(interrupt_data); > > > > > - kfree(interrupt_data->msix_entries); > > > > > +err_free_msix: > > > > > kfree(interrupt_data->eventfd_ctxs); > > > > > +err_free_eventfd: > > > > > + kfree(interrupt_data->msix_entries); > > > > > +err_free_data: > > > > > + kfree(interrupt_data); > > > > > return -ENOMEM; > > > > > } > > > > > > > > With this code, what will happen if, for example, the last > > > > allocation (interrupt_data->interrupt_counts) fails? > > > It took my some time but I think I saw it; the mistake is that > > > if interrupt_data->interrupt_counts fails it should 'goto > > > err_free_msix'. > > > > > > Same if interrupt_data->msix_entries fails it should 'goto > > > err_free_data'. > > > > > > Is that right? > > > > > > (I got very ambitious with the labels and I mixed it up ;) Any way to > > > make the goto labels make sense?) > > > > I don't think that the choice of labels is a problem. Basically, you > > should normally release things in the opposite order of the one in which > > they were allocated. At the beginning of your function, you can just do a > > return, because nothing has been allocated yet. Then when you have > > successfully allocated something, if you fail afterwards you should go to > > the very end of the function, because you have only one thing to free. > > When you have successfully allocated something else, if you fail > > afterwards, you should jump not quite to the end, so that you free both > > things that have been allocated, etc. > Indeed that is what I meant. Badly phrased but I released things the > wrong way around. I send a v3 of the patch (although as I just noticed > your latest comment, if that makes sense I am going to rebase against > greg's staging and resend it). Please send the version against Greg's tree. It would be easiest to see if the patch is correct if one can see the actual starting point. julia ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Outreachy kernel] [PATCH v2] staging: gasket Create a memory allocation path 2018-10-31 16:47 [PATCH v2] staging: gasket Create a memory allocation path Ioannis Valasakis 2018-10-31 16:54 ` [Outreachy kernel] " Sasha Levin @ 2018-10-31 20:40 ` Julia Lawall 2018-10-31 20:58 ` Ioannis Valasakis 1 sibling, 1 reply; 8+ messages in thread From: Julia Lawall @ 2018-10-31 20:40 UTC (permalink / raw) To: Ioannis Valasakis; +Cc: outreachy-kernel, gregkh, alexander.levin On Wed, 31 Oct 2018, Ioannis Valasakis wrote: > Fix multiple error allocation path by using re-direction. Beside the semantic problem that has already been pointed out, I find the word "re-direction" a bit strange, or a bit vague. What is redirected? Could it be something like "Collect the common error handling code at the end of the function"? I also have the impression that this patch is on top of your previous incorrect patch, instead of the original code. You should always work against the code in Greg's tree. julia > > Signed-off-by: Ioannis Valasakis <code@wizofe.uk> > --- > Changes in v2: > - Specific tags for memory allocation > - Free the memory in the right way as for kfre > > drivers/staging/gasket/gasket_interrupt.c | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/drivers/staging/gasket/gasket_interrupt.c b/drivers/staging/gasket/gasket_interrupt.c > index cdfb72af1c52..23232ea8f4d9 100644 > --- a/drivers/staging/gasket/gasket_interrupt.c > +++ b/drivers/staging/gasket/gasket_interrupt.c > @@ -348,21 +348,21 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev, const char *name, > sizeof(struct msix_entry), > GFP_KERNEL); > if (!interrupt_data->msix_entries) { > - goto err_alloc; > + goto err_free_msix; > } > > interrupt_data->eventfd_ctxs = kcalloc(num_interrupts, > sizeof(struct eventfd_ctx *), > GFP_KERNEL); > if (!interrupt_data->eventfd_ctxs) { > - goto err_alloc; > + goto err_free_eventfd; > } > > interrupt_data->interrupt_counts = kcalloc(num_interrupts, > sizeof(ulong), > GFP_KERNEL); > if (!interrupt_data->interrupt_counts) { > - goto err_alloc; > + goto err_free_data; > } > > switch (interrupt_data->type) { > @@ -397,10 +397,12 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev, const char *name, > > return 0; > > -err_alloc: > - kfree(interrupt_data); > - kfree(interrupt_data->msix_entries); > +err_free_msix: > kfree(interrupt_data->eventfd_ctxs); > +err_free_eventfd: > + kfree(interrupt_data->msix_entries); > +err_free_data: > + kfree(interrupt_data); > return -ENOMEM; > } > > -- > 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/20181031164753.GA14140%40kvasir.local. > For more options, visit https://groups.google.com/d/optout. > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Outreachy kernel] [PATCH v2] staging: gasket Create a memory allocation path 2018-10-31 20:40 ` Julia Lawall @ 2018-10-31 20:58 ` Ioannis Valasakis 0 siblings, 0 replies; 8+ messages in thread From: Ioannis Valasakis @ 2018-10-31 20:58 UTC (permalink / raw) To: Julia Lawall; +Cc: outreachy-kernel, gregkh, alexander.levin On Wed, Oct 31, 2018 at 09:40:47PM +0100, Julia Lawall wrote: > > > On Wed, 31 Oct 2018, Ioannis Valasakis wrote: > > > Fix multiple error allocation path by using re-direction. > > Beside the semantic problem that has already been pointed out, I find the > word "re-direction" a bit strange, or a bit vague. What is redirected? > Could it be something like "Collect the common error handling code at the > end of the function"? > > I also have the impression that this patch is on top of your previous > incorrect patch, instead of the original code. You should always work > against the code in Greg's tree. Thanks Julia. I am going to rebase on greg's staging and resend the patch (ah fourth one!). Sorry that I am bothering all of you but in every step I am learning something new. Thanks for being patient and helpful! > > julia > > > > > Signed-off-by: Ioannis Valasakis <code@wizofe.uk> > > --- > > Changes in v2: > > - Specific tags for memory allocation > > - Free the memory in the right way as for kfre > > > > drivers/staging/gasket/gasket_interrupt.c | 14 ++++++++------ > > 1 file changed, 8 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/staging/gasket/gasket_interrupt.c b/drivers/staging/gasket/gasket_interrupt.c > > index cdfb72af1c52..23232ea8f4d9 100644 > > --- a/drivers/staging/gasket/gasket_interrupt.c > > +++ b/drivers/staging/gasket/gasket_interrupt.c > > @@ -348,21 +348,21 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev, const char *name, > > sizeof(struct msix_entry), > > GFP_KERNEL); > > if (!interrupt_data->msix_entries) { > > - goto err_alloc; > > + goto err_free_msix; > > } > > > > interrupt_data->eventfd_ctxs = kcalloc(num_interrupts, > > sizeof(struct eventfd_ctx *), > > GFP_KERNEL); > > if (!interrupt_data->eventfd_ctxs) { > > - goto err_alloc; > > + goto err_free_eventfd; > > } > > > > interrupt_data->interrupt_counts = kcalloc(num_interrupts, > > sizeof(ulong), > > GFP_KERNEL); > > if (!interrupt_data->interrupt_counts) { > > - goto err_alloc; > > + goto err_free_data; > > } > > > > switch (interrupt_data->type) { > > @@ -397,10 +397,12 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev, const char *name, > > > > return 0; > > > > -err_alloc: > > - kfree(interrupt_data); > > - kfree(interrupt_data->msix_entries); > > +err_free_msix: > > kfree(interrupt_data->eventfd_ctxs); > > +err_free_eventfd: > > + kfree(interrupt_data->msix_entries); > > +err_free_data: > > + kfree(interrupt_data); > > return -ENOMEM; > > } > > > > -- > > 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/20181031164753.GA14140%40kvasir.local. > > For more options, visit https://groups.google.com/d/optout. > > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-10-31 21:04 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-10-31 16:47 [PATCH v2] staging: gasket Create a memory allocation path Ioannis Valasakis 2018-10-31 16:54 ` [Outreachy kernel] " Sasha Levin 2018-10-31 20:50 ` Ioannis Valasakis 2018-10-31 20:57 ` Julia Lawall 2018-10-31 21:00 ` Ioannis Valasakis 2018-10-31 21:04 ` Julia Lawall 2018-10-31 20:40 ` Julia Lawall 2018-10-31 20:58 ` Ioannis Valasakis
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.