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

[Finn Thain <fthain@telegraphics.com.au>]

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


On Mon, 11 May 2020, Markus Elfring wrote:

> > If you can't determine when the bug was introduced,
> 
> I might be able to determine also this information.
> 

This is tantamount to an admission of duplicity.

> 
> > how can you criticise a patch for the lack of a Fixes tag?
> 
> I dared to point two details out for the discussed patch.
> 

You deliberately chose those two details. You appear to be oblivious to 
your own motives.

> 
> >> To which commit would you like to refer to for the proposed 
> >> adjustment of the function “mac_sonic_platform_probe”?
> >
> > That was my question to you. We seem to be talking past each other.
> 
> We come along different views for this patch review. Who is going to add 
> a possible reference for this issue?
> 

Other opinions are not relevant: I was trying to communicate with you.

> 
> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?id=e99332e7b4cda6e60f5b5916cf9943a79dbef902#n460
> 
> >
> > My preference is unimportant here.
> 
> It is also relevant here because you added the tag “Reviewed-by”. 
> https://lore.kernel.org/patchwork/comment/1433193/ 
> https://lkml.org/lkml/2020/5/8/1827
> 

You have quoted my words out-of-context and twisted their meaning to suit 
your purposes.

> 
> > I presume that you mean to assert that Christophe's patch breaches the 
> > style guide.
> 
> I propose to take such a possibility into account.
> 

This "possibility" was among the reasons why the patch was posted to a 
mailing list by its author. That possibility is a given. If you claim this 
possibility as your motivation, you are being foolish or dishonest.

> 
> > However, 'sonic_probe1' is the name of a function.
> 
> The discussed source file does not contain such an identifier. 
> https://elixir.bootlin.com/linux/v5.7-rc5/source/drivers/net/ethernet/natsemi/macsonic.c#L486 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/natsemi/macsonic.c?id=2ef96a5bb12be62ef75b5828c0aab838ebb29cb8#n486
> 

That's what I told you in my previous email. You're welcome.

> 
> > This is not some sequence of GW-BASIC labels referred to in the style 
> > guide.
> 
> I recommend to read the current section “7) Centralized exiting of 
> functions” once more.
> 

Again, you are proposing a bike shed of a different color.

> 
> >> Can programming preferences evolve into the direction of “say what 
> >> the goto does”?
> >
> > I could agree that macsonic.c has no function resembling "probe1", and 
> > that portion of the patch could be improved.
> 
> I find this feedback interesting.
> 
> 
> > Was that the opinion you were trying to express by way of rhetorical 
> > questions? I can't tell.
> 
> Some known factors triggered my suggestion to consider the use of the 
> label “free_dma”.
> 

If you cannot express or convey your "known factors" then they aren't 
useful.

> 
> > Is it possible for a reviewer to effectively criticise C by use of 
> > English, when his C ability surpasses his English ability?
> 
> We come along possibly usual communication challenges.
> 

That looks like a machine translation. I can't make sense of it, sorry.

> Regards,
> Markus
> 

Markus, if you were to write a patch to improve upon coding-style.rst, who 
should review it?

If you are unable to write or review such a patch, how can you hope to 
adjudicate compliance?