[PATCH] net/sonic: Fix some resource leaks in error handling paths

[PATCH] net/sonic: Fix some resource leaks in error handling paths

[Christophe JAILLET]

A call to 'dma_alloc_coherent()' is hidden in 'sonic_alloc_descriptors()'.

This is correctly freed in the remove function, but not in the error
handling path of the probe function.
Fix it and add the missing 'dma_free_coherent()' call.

While at it, rename a label in order to be slightly more informative and
split some too long lines.

This patch is similar to commit 10e3cc180e64 ("net/sonic: Fix a resource leak in an error handling path in 'jazz_sonic_probe()'")
which was for 'jazzsonic.c'.

Suggested-by: Finn Thain <fthain@telegraphics.com.au>
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
Only macsonic has been compile tested. I don't have the needed setup to
compile xtsonic
---
 drivers/net/ethernet/natsemi/macsonic.c | 17 +++++++++++++----
 drivers/net/ethernet/natsemi/xtsonic.c  |  7 +++++--
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/natsemi/macsonic.c b/drivers/net/ethernet/natsemi/macsonic.c
index 1b5559aacb38..38d86c712bbc 100644
--- a/drivers/net/ethernet/natsemi/macsonic.c
+++ b/drivers/net/ethernet/natsemi/macsonic.c
@@ -506,10 +506,14 @@ static int mac_sonic_platform_probe(struct platform_device *pdev)
 
 	err = register_netdev(dev);
 	if (err)
-		goto out;
+		goto undo_probe1;
 
 	return 0;
 
+undo_probe1:
+	dma_free_coherent(lp->device,
+			  SIZEOF_SONIC_DESC * SONIC_BUS_SCALE(lp->dma_bitmode),
+			  lp->descriptors, lp->descriptors_laddr);
 out:
 	free_netdev(dev);
 
@@ -527,8 +531,9 @@ static int mac_sonic_platform_remove(struct platform_device *pdev)
 	struct sonic_local* lp = netdev_priv(dev);
 
 	unregister_netdev(dev);
-	dma_free_coherent(lp->device, SIZEOF_SONIC_DESC * SONIC_BUS_SCALE(lp->dma_bitmode),
-	                  lp->descriptors, lp->descriptors_laddr);
+	dma_free_coherent(lp->device,
+			  SIZEOF_SONIC_DESC * SONIC_BUS_SCALE(lp->dma_bitmode),
+			  lp->descriptors, lp->descriptors_laddr);
 	free_netdev(dev);
 
 	return 0;
@@ -584,12 +589,16 @@ static int mac_sonic_nubus_probe(struct nubus_board *board)
 
 	err = register_netdev(ndev);
 	if (err)
-		goto out;
+		goto undo_probe1;
 
 	nubus_set_drvdata(board, ndev);
 
 	return 0;
 
+undo_probe1:
+	dma_free_coherent(lp->device,
+			  SIZEOF_SONIC_DESC * SONIC_BUS_SCALE(lp->dma_bitmode),
+			  lp->descriptors, lp->descriptors_laddr);
 out:
 	free_netdev(ndev);
 	return err;
diff --git a/drivers/net/ethernet/natsemi/xtsonic.c b/drivers/net/ethernet/natsemi/xtsonic.c
index dda9ec7d9cee..a917f1a830fc 100644
--- a/drivers/net/ethernet/natsemi/xtsonic.c
+++ b/drivers/net/ethernet/natsemi/xtsonic.c
@@ -229,11 +229,14 @@ int xtsonic_probe(struct platform_device *pdev)
 	sonic_msg_init(dev);
 
 	if ((err = register_netdev(dev)))
-		goto out1;
+		goto undo_probe1;
 
 	return 0;
 
-out1:
+undo_probe1:
+	dma_free_coherent(lp->device,
+			  SIZEOF_SONIC_DESC * SONIC_BUS_SCALE(lp->dma_bitmode),
+			  lp->descriptors, lp->descriptors_laddr);
 	release_region(dev->base_addr, SONIC_MEM_SIZE);
 out:
 	free_netdev(dev);
-- 
2.25.1

Re: [PATCH] net/sonic: Fix some resource leaks in error handling paths

[Finn Thain]

On Fri, 8 May 2020, Christophe JAILLET wrote:

> A call to 'dma_alloc_coherent()' is hidden in 
> 'sonic_alloc_descriptors()'.
> 
> This is correctly freed in the remove function, but not in the error 
> handling path of the probe function. Fix it and add the missing 
> 'dma_free_coherent()' call.
> 
> While at it, rename a label in order to be slightly more informative and 
> split some too long lines.
> 
> This patch is similar to commit 10e3cc180e64 ("net/sonic: Fix a resource 
> leak in an error handling path in 'jazz_sonic_probe()'") which was for 
> 'jazzsonic.c'.
> 
> Suggested-by: Finn Thain <fthain@telegraphics.com.au>
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>

Thanks.

Reviewed-by: Finn Thain <fthain@telegraphics.com.au>

> ---
> Only macsonic has been compile tested. I don't have the needed setup to
> compile xtsonic

I compile tested xtsonic.c. I didn't use an xtensa toolchain as there's 
probably no need: the new code already appears elsewhere in that file.

Re: [PATCH] net/sonic: Fix some resource leaks in error handling paths

[Jakub Kicinski]

On Fri,  8 May 2020 19:25:57 +0200 Christophe JAILLET wrote:
> Only macsonic has been compile tested. I don't have the needed setup to
> compile xtsonic

Well, we gotta do that before we apply the patch :S

Does the driver actually depend on some platform stuff, or can we 
do this:

diff --git a/drivers/net/ethernet/natsemi/Kconfig b/drivers/net/ethernet/natsemi/Kconfig
@@ -58,7 +58,7 @@ config NS83820
 
 config XTENSA_XT2000_SONIC
        tristate "Xtensa XT2000 onboard SONIC Ethernet support"
-       depends on XTENSA_PLATFORM_XT2000
+       depends on XTENSA_PLATFORM_XT2000 || COMPILE_TEST
        ---help---
          This is the driver for the onboard card of the Xtensa XT2000 board.
 
?

Re: [PATCH] net/sonic: Fix some resource leaks in error handling paths

[Jakub Kicinski]

On Fri,  8 May 2020 19:25:57 +0200 Christophe JAILLET wrote:
> @@ -527,8 +531,9 @@ static int mac_sonic_platform_remove(struct platform_device *pdev)
>  	struct sonic_local* lp = netdev_priv(dev);
>  
>  	unregister_netdev(dev);
> -	dma_free_coherent(lp->device, SIZEOF_SONIC_DESC * SONIC_BUS_SCALE(lp->dma_bitmode),
> -	                  lp->descriptors, lp->descriptors_laddr);
> +	dma_free_coherent(lp->device,
> +			  SIZEOF_SONIC_DESC * SONIC_BUS_SCALE(lp->dma_bitmode),
> +			  lp->descriptors, lp->descriptors_laddr);
>  	free_netdev(dev);
>  
>  	return 0;

This is a white-space only change, right? Since this is a fix we should
avoid making cleanups which are not strictly necessary.

Re: [PATCH] net/sonic: Fix some resource leaks in error handling paths

[Finn Thain]

On Fri, 8 May 2020, Jakub Kicinski wrote:

> On Fri,  8 May 2020 19:25:57 +0200 Christophe JAILLET wrote:
> > Only macsonic has been compile tested. I don't have the needed setup to
> > compile xtsonic
> 
> Well, we gotta do that before we apply the patch :S
> 

I've compiled xtsonic.c with this patch.

> Does the driver actually depend on some platform stuff,

xtsonic.c looks portable enough but it has some asm includes that I 
haven't looked at. It is really a question for the arch maintainers.

>  or can we do this:
> 
> diff --git a/drivers/net/ethernet/natsemi/Kconfig b/drivers/net/ethernet/natsemi/Kconfig
> @@ -58,7 +58,7 @@ config NS83820
>  
>  config XTENSA_XT2000_SONIC
>         tristate "Xtensa XT2000 onboard SONIC Ethernet support"
> -       depends on XTENSA_PLATFORM_XT2000
> +       depends on XTENSA_PLATFORM_XT2000 || COMPILE_TEST
>         ---help---
>           This is the driver for the onboard card of the Xtensa XT2000 board.
>  
> ?
> 

That's effectively what I did to compile test xtsonic.c (I removed the 
line to get the same effect).

Re: [PATCH] net/sonic: Fix some resource leaks in error handling paths

[Jakub Kicinski]

On Sat, 9 May 2020 11:57:44 +1000 (AEST) Finn Thain wrote:
> On Fri, 8 May 2020, Jakub Kicinski wrote:
> > On Fri,  8 May 2020 19:25:57 +0200 Christophe JAILLET wrote:  
> > > Only macsonic has been compile tested. I don't have the needed setup to
> > > compile xtsonic  
> > 
> > Well, we gotta do that before we apply the patch :S
> >   
> 
> I've compiled xtsonic.c with this patch.
> 
> > Does the driver actually depend on some platform stuff,  
> 
> xtsonic.c looks portable enough but it has some asm includes that I 
> haven't looked at. It is really a question for the arch maintainers.

I see.

> >  or can we do this:
> > 
> > diff --git a/drivers/net/ethernet/natsemi/Kconfig b/drivers/net/ethernet/natsemi/Kconfig
> > @@ -58,7 +58,7 @@ config NS83820
> >  
> >  config XTENSA_XT2000_SONIC
> >         tristate "Xtensa XT2000 onboard SONIC Ethernet support"
> > -       depends on XTENSA_PLATFORM_XT2000
> > +       depends on XTENSA_PLATFORM_XT2000 || COMPILE_TEST
> >         ---help---
> >           This is the driver for the onboard card of the Xtensa XT2000 board.
> >  
> > ?
> >   
> 
> That's effectively what I did to compile test xtsonic.c (I removed the 
> line to get the same effect).

Thank you, that should do!

Re: [PATCH] net/sonic: Fix some resource leaks in error handling paths

[Christophe JAILLET]

Le 09/05/2020 à 03:54, Jakub Kicinski a écrit :
> On Fri,  8 May 2020 19:25:57 +0200 Christophe JAILLET wrote:
>> @@ -527,8 +531,9 @@ static int mac_sonic_platform_remove(struct platform_device *pdev)
>>   	struct sonic_local* lp = netdev_priv(dev);
>>   
>>   	unregister_netdev(dev);
>> -	dma_free_coherent(lp->device, SIZEOF_SONIC_DESC * SONIC_BUS_SCALE(lp->dma_bitmode),
>> -	                  lp->descriptors, lp->descriptors_laddr);
>> +	dma_free_coherent(lp->device,
>> +			  SIZEOF_SONIC_DESC * SONIC_BUS_SCALE(lp->dma_bitmode),
>> +			  lp->descriptors, lp->descriptors_laddr);
>>   	free_netdev(dev);
>>   
>>   	return 0;
> This is a white-space only change, right? Since this is a fix we should
> avoid making cleanups which are not strictly necessary.
>
Right.

The reason of this clean-up is that I wanted to avoid a checkpatch 
warning with the proposed patch and I felt that having the same layout 
in the error handling path of the probe function and in the remove 
function was clearer.
So I updated also the remove function.

Fell free to ignore this hunk if not desired. I will not sent a V2 only 
for that.

CJ

Re: [PATCH] net/sonic: Fix some resource leaks in error handling paths

[Jakub Kicinski]

On Sat, 9 May 2020 18:47:08 +0200 Christophe JAILLET wrote:
> Le 09/05/2020 à 03:54, Jakub Kicinski a écrit :
> > On Fri,  8 May 2020 19:25:57 +0200 Christophe JAILLET wrote:  
> >> @@ -527,8 +531,9 @@ static int mac_sonic_platform_remove(struct platform_device *pdev)
> >>   	struct sonic_local* lp = netdev_priv(dev);
> >>   
> >>   	unregister_netdev(dev);
> >> -	dma_free_coherent(lp->device, SIZEOF_SONIC_DESC * SONIC_BUS_SCALE(lp->dma_bitmode),
> >> -	                  lp->descriptors, lp->descriptors_laddr);
> >> +	dma_free_coherent(lp->device,
> >> +			  SIZEOF_SONIC_DESC * SONIC_BUS_SCALE(lp->dma_bitmode),
> >> +			  lp->descriptors, lp->descriptors_laddr);
> >>   	free_netdev(dev);
> >>   
> >>   	return 0;  
> > This is a white-space only change, right? Since this is a fix we should
> > avoid making cleanups which are not strictly necessary.
>
> Right.
> 
> The reason of this clean-up is that I wanted to avoid a checkpatch 
> warning with the proposed patch and I felt that having the same layout 
> in the error handling path of the probe function and in the remove 
> function was clearer.
> So I updated also the remove function.

I understand the motivation is good.

> Fell free to ignore this hunk if not desired. I will not sent a V2 only 
> for that.

That's not how it works. Busy maintainers don't have time to hand edit
patches. I'm not applying this to the networking tree and I'm tossing it
from patchwork. Please address the basic feedback.

Thank you.

Re: [PATCH] net/sonic: Fix some resource leaks in error handling paths

[Christophe JAILLET]

Le 09/05/2020 à 20:13, Jakub Kicinski a écrit :
> On Sat, 9 May 2020 18:47:08 +0200 Christophe JAILLET wrote:
>> Le 09/05/2020 à 03:54, Jakub Kicinski a écrit :
>>> On Fri,  8 May 2020 19:25:57 +0200 Christophe JAILLET wrote:
>>>> @@ -527,8 +531,9 @@ static int mac_sonic_platform_remove(struct platform_device *pdev)
>>>>    	struct sonic_local* lp = netdev_priv(dev);
>>>>    
>>>>    	unregister_netdev(dev);
>>>> -	dma_free_coherent(lp->device, SIZEOF_SONIC_DESC * SONIC_BUS_SCALE(lp->dma_bitmode),
>>>> -	                  lp->descriptors, lp->descriptors_laddr);
>>>> +	dma_free_coherent(lp->device,
>>>> +			  SIZEOF_SONIC_DESC * SONIC_BUS_SCALE(lp->dma_bitmode),
>>>> +			  lp->descriptors, lp->descriptors_laddr);
>>>>    	free_netdev(dev);
>>>>    
>>>>    	return 0;
>>> This is a white-space only change, right? Since this is a fix we should
>>> avoid making cleanups which are not strictly necessary.
>> Right.
>>
>> The reason of this clean-up is that I wanted to avoid a checkpatch
>> warning with the proposed patch and I felt that having the same layout
>> in the error handling path of the probe function and in the remove
>> function was clearer.
>> So I updated also the remove function.
> I understand the motivation is good.
>
>> Fell free to ignore this hunk if not desired. I will not sent a V2 only
>> for that.
> That's not how it works. Busy maintainers don't have time to hand edit
> patches. I'm not applying this to the networking tree and I'm tossing it
> from patchwork. Please address the basic feedback.
>
> Thank you.
>
Hi,

that's not the way you would like it to work.
It happens that some maintainers make some small adjustments in the 
commit message or the patch itself.

The patch is good enough for me. If you can not accept the additional 
small clean-up, or don't have time to tweak it by yourself, or by anyone 
else, please, just reject it.
The issue I propose to fix is minor and unlikely to happen anyway.

If anyone else cares to update the proposal, please do.


I don't want to discuss your motivation, I understand them.

But please, do also understand mine and do not require too futile things 
from hobbyists.

Spending time only to remove a CR because it does not match your quality 
standard or your expectation of what a patch is, is of no interest for me.
That's why I told I would not send a V2.


It is up to you to accept it as-is, update it or reject it, according to 
the value you think this patch has.

Hoping for your understanding and sorry for wasting your time.

Best regards,
CJ

Re: [PATCH] net/sonic: Fix some resource leaks in error handling paths

[Joe Perches]

On Sat, 2020-05-09 at 11:13 -0700, Jakub Kicinski wrote:
> On Sat, 9 May 2020 18:47:08 +0200 Christophe JAILLET wrote:
> > Le 09/05/2020 à 03:54, Jakub Kicinski a écrit :
> > > On Fri,  8 May 2020 19:25:57 +0200 Christophe JAILLET wrote:  
> > > > @@ -527,8 +531,9 @@ static int mac_sonic_platform_remove(struct platform_device *pdev)
> > > >   	struct sonic_local* lp = netdev_priv(dev);
> > > >   
> > > >   	unregister_netdev(dev);
> > > > -	dma_free_coherent(lp->device, SIZEOF_SONIC_DESC * SONIC_BUS_SCALE(lp->dma_bitmode),
> > > > -	                  lp->descriptors, lp->descriptors_laddr);
> > > > +	dma_free_coherent(lp->device,
> > > > +			  SIZEOF_SONIC_DESC * SONIC_BUS_SCALE(lp->dma_bitmode),
> > > > +			  lp->descriptors, lp->descriptors_laddr);
> > > >   	free_netdev(dev);
> > > >   
> > > >   	return 0;  
> > > This is a white-space only change, right? Since this is a fix we should
> > > avoid making cleanups which are not strictly necessary.
> > 
> > Right.
> > 
> > The reason of this clean-up is that I wanted to avoid a checkpatch 
> > warning with the proposed patch and I felt that having the same layout 
> > in the error handling path of the probe function and in the remove 
> > function was clearer.
> > So I updated also the remove function.
> 
> I understand the motivation is good.

David, maybe I missed some notification about Jakub's role.

What is Jakub's role in relation to the networking tree?

Re: [PATCH] net/sonic: Fix some resource leaks in error handling paths

[David Miller]

From: Joe Perches <joe@perches.com>
Date: Sat, 09 May 2020 15:42:36 -0700

> David, maybe I missed some notification about Jakub's role.
> 
> What is Jakub's role in relation to the networking tree?

He is the co-maintainer of the networking tree and you should respect
his actions and feedback as if it were coming from me.

Re: [PATCH] net/sonic: Fix some resource leaks in error handling paths

[Joe Perches]

On Sat, 2020-05-09 at 16:32 -0700, David Miller wrote:
> From: Joe Perches <joe@perches.com>
> Date: Sat, 09 May 2020 15:42:36 -0700
> 
> > David, maybe I missed some notification about Jakub's role.
> > 
> > What is Jakub's role in relation to the networking tree?
> 
> He is the co-maintainer of the networking tree and you should respect
> his actions and feedback as if it were coming from me.

If he's committing drivers then presumably then
he should be added to this section as well.

NETWORKING DRIVERS
M:	"David S. Miller" <davem@davemloft.net>
L:	netdev@vger.kernel.org
S:	Odd Fixes

Re: [PATCH] net/sonic: Fix some resource leaks in error handling paths

[Finn Thain]

On Fri, 8 May 2020, Jakub Kicinski wrote:

> On Fri,  8 May 2020 19:25:57 +0200 Christophe JAILLET wrote:
> > @@ -527,8 +531,9 @@ static int mac_sonic_platform_remove(struct platform_device *pdev)
> >  	struct sonic_local* lp = netdev_priv(dev);
> >  
> >  	unregister_netdev(dev);
> > -	dma_free_coherent(lp->device, SIZEOF_SONIC_DESC * SONIC_BUS_SCALE(lp->dma_bitmode),
> > -	                  lp->descriptors, lp->descriptors_laddr);
> > +	dma_free_coherent(lp->device,
> > +			  SIZEOF_SONIC_DESC * SONIC_BUS_SCALE(lp->dma_bitmode),
> > +			  lp->descriptors, lp->descriptors_laddr);
> >  	free_netdev(dev);
> >  
> >  	return 0;
> 
> This is a white-space only change, right? Since this is a fix we should
> avoid making cleanups which are not strictly necessary.
> 

I think it is harmless if it doesn't create any merge conflicts. Any merge 
conflict created by the whitespace change would have happened anyway, 
because all of the changes in this patch are very closely related. That's 
why I was happy to put a 'reviewed-by' tag on this.

Re: [PATCH] net/sonic: Fix some resource leaks in error handling paths

[Finn Thain]

[-- Attachment #1: Type: text/plain, Size: 1743 bytes --]

On Sun, 10 May 2020, Markus Elfring wrote:

> Christophe Jaillet proposed to complete the exception handling also for this
> function implementation.
> I find that such a software correction is qualified for this tag.
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=e99332e7b4cda6e60f5b5916cf9943a79dbef902#n183
> 
> Corresponding consequences can vary then according to the change 
> management of involved developers.
> 

Makes sense.

> > I think 'undo_probe1' is both descriptive and consistent with commit 
> > 10e3cc180e64 ("net/sonic: Fix a resource leak in an error handling 
> > path in 'jazz_sonic_probe()'").
> 
> I can agree to this view (in principle).
> 
> By the way:
> The referenced commit contains the tag “Fixes”.
> https://lore.kernel.org/patchwork/patch/1231354/
> https://lore.kernel.org/lkml/20200427061803.53857-1-christophe.jaillet@wanadoo.fr/
> 

Right, I'd forgotten that. Do you know when these bugs were introduced?

> > Your suggestion, 'free_dma' is also good.
> 
> Thanks for your positive feedback.
> 
> 
> > But coming up with good alternatives is easy.
> 
> But the change acceptance can occasionally become harder.
> 

The path to patch acceptance often takes surprising turns.

> 
> > If every good alternative would be considered there would be no 
> > obvious way to get a patch merged.
> 
> I imagine that some alternatives can result in preferable solutions, 
> can't they?

Naming goto labels is just painting another bikeshed. Yes, some 
alternatives are preferable but it takes too long to identify them and 
finding consensus is unlikely anyway, as it's a matter of taste.

Re: [PATCH] net/sonic: Fix some resource leaks in error handling paths

[Markus Elfring]

> Is there a way to add a Fixes tag that would not invoke the -stable
> process? And was that what you had in mind?

Christophe Jaillet proposed to complete the exception handling also for this
function implementation.
I find that such a software correction is qualified for this tag.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=e99332e7b4cda6e60f5b5916cf9943a79dbef902#n183

Corresponding consequences can vary then according to the change management
of involved developers.


> I think 'undo_probe1' is both descriptive and consistent with commit
> 10e3cc180e64 ("net/sonic: Fix a resource leak in an error handling path in
> 'jazz_sonic_probe()'").

I can agree to this view (in principle).

By the way:
The referenced commit contains the tag “Fixes”.
https://lore.kernel.org/patchwork/patch/1231354/
https://lore.kernel.org/lkml/20200427061803.53857-1-christophe.jaillet@wanadoo.fr/


> Your suggestion, 'free_dma' is also good.

Thanks for your positive feedback.


> But coming up with good alternatives is easy.

But the change acceptance can occasionally become harder.


> If every good alternative would be considered there would be no obvious way
> to get a patch merged.

I imagine that some alternatives can result in preferable solutions, can't they?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?id=e99332e7b4cda6e60f5b5916cf9943a79dbef902#n460

Regards,
Markus

Re: [PATCH] net/sonic: Fix some resource leaks in error handling paths

[Finn Thain]

On Sat, 9 May 2020, Markus Elfring wrote:

> > While at it, rename a label in order to be slightly more informative and
> > split some too long lines.
> 
> Would you like to add the tag 'Fixes' to the change description?
> 

Sorry but I don't follow your reasoning here. Are you saying that this 
needs to be pushed out to -stable branches? If so, stable-kernel-rules.rst 
would seem to disagree as the bug is theoretical and isn't bothering 
people.

Is there a way to add a Fixes tag that would not invoke the -stable 
process? And was that what you had in mind?

> 
> > +++ b/drivers/net/ethernet/natsemi/macsonic.c
> > @@ -506,10 +506,14 @@ static int mac_sonic_platform_probe(struct platform_device *pdev)
> >
> >  	err = register_netdev(dev);
> >  	if (err)
> > -		goto out;
> > +		goto undo_probe1;
> >
> >  	return 0;
> >
> > +undo_probe1:
> > +	dma_free_coherent(lp->device,
> > +			  SIZEOF_SONIC_DESC * SONIC_BUS_SCALE(lp->dma_bitmode),
> > +			  lp->descriptors, lp->descriptors_laddr);
> >  out:
> How do you think about the possibility to use the label 'free_dma'?

I think 'undo_probe1' is both descriptive and consistent with commit 
10e3cc180e64 ("net/sonic: Fix a resource leak in an error handling path in 
'jazz_sonic_probe()'").

Your suggestion, 'free_dma' is also good. But coming up with good 
alternatives is easy. If every good alternative would be considered there 
would be no obvious way to get a patch merged.

Re: [PATCH] net/sonic: Fix some resource leaks in error handling paths

[Markus Elfring]

> While at it, rename a label in order to be slightly more informative and
> split some too long lines.

Would you like to add the tag “Fixes” to the change description?


…
> +++ b/drivers/net/ethernet/natsemi/macsonic.c
> @@ -506,10 +506,14 @@ static int mac_sonic_platform_probe(struct platform_device *pdev)
>
>  	err = register_netdev(dev);
>  	if (err)
> -		goto out;
> +		goto undo_probe1;
>
>  	return 0;
>
> +undo_probe1:
> +	dma_free_coherent(lp->device,
> +			  SIZEOF_SONIC_DESC * SONIC_BUS_SCALE(lp->dma_bitmode),
> +			  lp->descriptors, lp->descriptors_laddr);
>  out:
…

How do you think about the possibility to use the label “free_dma”?

Regards,
Markus