linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] PCI: tegra: Fix OF node reference leak
@ 2021-05-04 17:17 Christophe JAILLET
  2021-05-04 17:17 ` [PATCH 2/3] PCI: tegra: Use 'seq_puts' instead of 'seq_printf' Christophe JAILLET
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Christophe JAILLET @ 2021-05-04 17:17 UTC (permalink / raw)
  To: thierry.reding, lorenzo.pieralisi, robh, bhelgaas, jonathanh
  Cc: linux-tegra, linux-pci, linux-kernel, kernel-janitors,
	Christophe JAILLET

Commit 9e38e690ace3 ("PCI: tegra: Fix OF node reference leak") has fixed
some node reference leaks in this function but missed some of them.

In fact, having 'port' referenced in the 'rp' structure is not enough to
prevent the leak, until 'rp' is actually added in the 'pcie->ports' list.

Add the missing 'goto err_node_put' accordingly.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
 drivers/pci/controller/pci-tegra.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c
index 8069bd9232d4..006bf0346dec 100644
--- a/drivers/pci/controller/pci-tegra.c
+++ b/drivers/pci/controller/pci-tegra.c
@@ -2193,13 +2193,15 @@ static int tegra_pcie_parse_dt(struct tegra_pcie *pcie)
 		rp->np = port;
 
 		rp->base = devm_pci_remap_cfg_resource(dev, &rp->regs);
-		if (IS_ERR(rp->base))
-			return PTR_ERR(rp->base);
+		if (IS_ERR(rp->base)) {
+			err = PTR_ERR(rp->base);
+			goto err_node_put;
+		}
 
 		label = devm_kasprintf(dev, GFP_KERNEL, "pex-reset-%u", index);
 		if (!label) {
-			dev_err(dev, "failed to create reset GPIO label\n");
-			return -ENOMEM;
+			err = -ENOMEM;
+			goto err_node_put;
 		}
 
 		/*
@@ -2217,7 +2219,8 @@ static int tegra_pcie_parse_dt(struct tegra_pcie *pcie)
 			} else {
 				dev_err(dev, "failed to get reset GPIO: %ld\n",
 					PTR_ERR(rp->reset_gpio));
-				return PTR_ERR(rp->reset_gpio);
+				err = PTR_ERR(rp->reset_gpio);
+				goto err_node_put;
 			}
 		}
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/3] PCI: tegra: Use 'seq_puts' instead of 'seq_printf'
  2021-05-04 17:17 [PATCH 1/3] PCI: tegra: Fix OF node reference leak Christophe JAILLET
@ 2021-05-04 17:17 ` Christophe JAILLET
  2021-07-05 17:01   ` Vidya Sagar
  2021-05-04 17:18 ` [PATCH 3/3] PCI: tegra: make const array err_msg static Christophe JAILLET
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Christophe JAILLET @ 2021-05-04 17:17 UTC (permalink / raw)
  To: thierry.reding, lorenzo.pieralisi, robh, bhelgaas, jonathanh
  Cc: linux-tegra, linux-pci, linux-kernel, kernel-janitors,
	Christophe JAILLET

As spotted by checkpatch, use 'seq_puts' instead of 'seq_printf' when
possible.
It is slightly more efficient.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
 drivers/pci/controller/pci-tegra.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c
index 006bf0346dec..fe8e21ce3e3d 100644
--- a/drivers/pci/controller/pci-tegra.c
+++ b/drivers/pci/controller/pci-tegra.c
@@ -2550,7 +2550,7 @@ static void *tegra_pcie_ports_seq_start(struct seq_file *s, loff_t *pos)
 	if (list_empty(&pcie->ports))
 		return NULL;
 
-	seq_printf(s, "Index  Status\n");
+	seq_puts(s, "Index  Status\n");
 
 	return seq_list_start(&pcie->ports, *pos);
 }
@@ -2587,16 +2587,16 @@ static int tegra_pcie_ports_seq_show(struct seq_file *s, void *v)
 	seq_printf(s, "%2u     ", port->index);
 
 	if (up)
-		seq_printf(s, "up");
+		seq_puts(s, "up");
 
 	if (active) {
 		if (up)
-			seq_printf(s, ", ");
+			seq_puts(s, ", ");
 
-		seq_printf(s, "active");
+		seq_puts(s, "active");
 	}
 
-	seq_printf(s, "\n");
+	seq_puts(s, "\n");
 	return 0;
 }
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/3] PCI: tegra: make const array err_msg static
  2021-05-04 17:17 [PATCH 1/3] PCI: tegra: Fix OF node reference leak Christophe JAILLET
  2021-05-04 17:17 ` [PATCH 2/3] PCI: tegra: Use 'seq_puts' instead of 'seq_printf' Christophe JAILLET
@ 2021-05-04 17:18 ` Christophe JAILLET
  2021-07-05 17:01   ` Vidya Sagar
  2021-07-05 22:31   ` Krzysztof Wilczyński
  2021-06-22 10:42 ` [PATCH 1/3] PCI: tegra: Fix OF node reference leak Lorenzo Pieralisi
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 11+ messages in thread
From: Christophe JAILLET @ 2021-05-04 17:18 UTC (permalink / raw)
  To: thierry.reding, lorenzo.pieralisi, robh, bhelgaas, jonathanh
  Cc: linux-tegra, linux-pci, linux-kernel, kernel-janitors,
	Christophe JAILLET

Don't populate the array err_msg on the stack but instead make it
static. Makes the object code smaller by 64 bytes.

While at it, add a missing const, as reported by checkpatch.

Compiled with gcc 11.0.1

Before:
$ size drivers/pci/controller/pci-tegra.o
   text	   data	    bss	    dec	    hex	filename
  25623	   2844	     32	  28499	   6f53	drivers/pci/controller/pci-tegra.o

After:
$ size drivers/pci/controller/pci-tegra.o
   text	   data	    bss	    dec	    hex	filename
  25559	   2844	     32	  28435	   6f13	drivers/pci/controller/pci-tegra.o

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
 drivers/pci/controller/pci-tegra.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c
index fe8e21ce3e3d..b1250b15d290 100644
--- a/drivers/pci/controller/pci-tegra.c
+++ b/drivers/pci/controller/pci-tegra.c
@@ -764,7 +764,7 @@ static int tegra_pcie_map_irq(const struct pci_dev *pdev, u8 slot, u8 pin)
 
 static irqreturn_t tegra_pcie_isr(int irq, void *arg)
 {
-	const char *err_msg[] = {
+	static const char * const err_msg[] = {
 		"Unknown",
 		"AXI slave error",
 		"AXI decode error",
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] PCI: tegra: Fix OF node reference leak
  2021-05-04 17:17 [PATCH 1/3] PCI: tegra: Fix OF node reference leak Christophe JAILLET
  2021-05-04 17:17 ` [PATCH 2/3] PCI: tegra: Use 'seq_puts' instead of 'seq_printf' Christophe JAILLET
  2021-05-04 17:18 ` [PATCH 3/3] PCI: tegra: make const array err_msg static Christophe JAILLET
@ 2021-06-22 10:42 ` Lorenzo Pieralisi
  2021-07-05 17:00 ` Vidya Sagar
  2021-08-05 10:43 ` Lorenzo Pieralisi
  4 siblings, 0 replies; 11+ messages in thread
From: Lorenzo Pieralisi @ 2021-06-22 10:42 UTC (permalink / raw)
  To: Christophe JAILLET, thierry.reding
  Cc: robh, bhelgaas, jonathanh, linux-tegra, linux-pci, linux-kernel,
	kernel-janitors, vidyas

On Tue, May 04, 2021 at 07:17:42PM +0200, Christophe JAILLET wrote:
> Commit 9e38e690ace3 ("PCI: tegra: Fix OF node reference leak") has fixed
> some node reference leaks in this function but missed some of them.
> 
> In fact, having 'port' referenced in the 'rp' structure is not enough to
> prevent the leak, until 'rp' is actually added in the 'pcie->ports' list.
> 
> Add the missing 'goto err_node_put' accordingly.
> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
>  drivers/pci/controller/pci-tegra.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)

Thierry, Jon, Vidya,

please review this series when you have time, thanks.

Lorenzo

> diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c
> index 8069bd9232d4..006bf0346dec 100644
> --- a/drivers/pci/controller/pci-tegra.c
> +++ b/drivers/pci/controller/pci-tegra.c
> @@ -2193,13 +2193,15 @@ static int tegra_pcie_parse_dt(struct tegra_pcie *pcie)
>  		rp->np = port;
>  
>  		rp->base = devm_pci_remap_cfg_resource(dev, &rp->regs);
> -		if (IS_ERR(rp->base))
> -			return PTR_ERR(rp->base);
> +		if (IS_ERR(rp->base)) {
> +			err = PTR_ERR(rp->base);
> +			goto err_node_put;
> +		}
>  
>  		label = devm_kasprintf(dev, GFP_KERNEL, "pex-reset-%u", index);
>  		if (!label) {
> -			dev_err(dev, "failed to create reset GPIO label\n");
> -			return -ENOMEM;
> +			err = -ENOMEM;
> +			goto err_node_put;
>  		}
>  
>  		/*
> @@ -2217,7 +2219,8 @@ static int tegra_pcie_parse_dt(struct tegra_pcie *pcie)
>  			} else {
>  				dev_err(dev, "failed to get reset GPIO: %ld\n",
>  					PTR_ERR(rp->reset_gpio));
> -				return PTR_ERR(rp->reset_gpio);
> +				err = PTR_ERR(rp->reset_gpio);
> +				goto err_node_put;
>  			}
>  		}
>  
> -- 
> 2.30.2
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] PCI: tegra: Fix OF node reference leak
  2021-05-04 17:17 [PATCH 1/3] PCI: tegra: Fix OF node reference leak Christophe JAILLET
                   ` (2 preceding siblings ...)
  2021-06-22 10:42 ` [PATCH 1/3] PCI: tegra: Fix OF node reference leak Lorenzo Pieralisi
@ 2021-07-05 17:00 ` Vidya Sagar
  2021-08-05 10:43 ` Lorenzo Pieralisi
  4 siblings, 0 replies; 11+ messages in thread
From: Vidya Sagar @ 2021-07-05 17:00 UTC (permalink / raw)
  To: Christophe JAILLET, thierry.reding, lorenzo.pieralisi, robh,
	bhelgaas, jonathanh
  Cc: linux-tegra, linux-pci, linux-kernel, kernel-janitors

Reviewed-by: Vidya Sagar <vidyas@nvidia.com>

On 5/4/2021 10:47 PM, Christophe JAILLET wrote:
> Commit 9e38e690ace3 ("PCI: tegra: Fix OF node reference leak") has fixed
> some node reference leaks in this function but missed some of them.
> 
> In fact, having 'port' referenced in the 'rp' structure is not enough to
> prevent the leak, until 'rp' is actually added in the 'pcie->ports' list.
> 
> Add the missing 'goto err_node_put' accordingly.
> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
>   drivers/pci/controller/pci-tegra.c | 13 ++++++++-----
>   1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c
> index 8069bd9232d4..006bf0346dec 100644
> --- a/drivers/pci/controller/pci-tegra.c
> +++ b/drivers/pci/controller/pci-tegra.c
> @@ -2193,13 +2193,15 @@ static int tegra_pcie_parse_dt(struct tegra_pcie *pcie)
>   		rp->np = port;
>   
>   		rp->base = devm_pci_remap_cfg_resource(dev, &rp->regs);
> -		if (IS_ERR(rp->base))
> -			return PTR_ERR(rp->base);
> +		if (IS_ERR(rp->base)) {
> +			err = PTR_ERR(rp->base);
> +			goto err_node_put;
> +		}
>   
>   		label = devm_kasprintf(dev, GFP_KERNEL, "pex-reset-%u", index);
>   		if (!label) {
> -			dev_err(dev, "failed to create reset GPIO label\n");
> -			return -ENOMEM;
> +			err = -ENOMEM;
> +			goto err_node_put;
>   		}
>   
>   		/*
> @@ -2217,7 +2219,8 @@ static int tegra_pcie_parse_dt(struct tegra_pcie *pcie)
>   			} else {
>   				dev_err(dev, "failed to get reset GPIO: %ld\n",
>   					PTR_ERR(rp->reset_gpio));
> -				return PTR_ERR(rp->reset_gpio);
> +				err = PTR_ERR(rp->reset_gpio);
> +				goto err_node_put;
>   			}
>   		}
>   
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] PCI: tegra: Use 'seq_puts' instead of 'seq_printf'
  2021-05-04 17:17 ` [PATCH 2/3] PCI: tegra: Use 'seq_puts' instead of 'seq_printf' Christophe JAILLET
@ 2021-07-05 17:01   ` Vidya Sagar
  0 siblings, 0 replies; 11+ messages in thread
From: Vidya Sagar @ 2021-07-05 17:01 UTC (permalink / raw)
  To: Christophe JAILLET, thierry.reding, lorenzo.pieralisi, robh,
	bhelgaas, jonathanh
  Cc: linux-tegra, linux-pci, linux-kernel, kernel-janitors

Reviewed-by: Vidya Sagar <vidyas@nvidia.com>

On 5/4/2021 10:47 PM, Christophe JAILLET wrote:
> As spotted by checkpatch, use 'seq_puts' instead of 'seq_printf' when
> possible.
> It is slightly more efficient.
> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
>   drivers/pci/controller/pci-tegra.c | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c
> index 006bf0346dec..fe8e21ce3e3d 100644
> --- a/drivers/pci/controller/pci-tegra.c
> +++ b/drivers/pci/controller/pci-tegra.c
> @@ -2550,7 +2550,7 @@ static void *tegra_pcie_ports_seq_start(struct seq_file *s, loff_t *pos)
>   	if (list_empty(&pcie->ports))
>   		return NULL;
>   
> -	seq_printf(s, "Index  Status\n");
> +	seq_puts(s, "Index  Status\n");
>   
>   	return seq_list_start(&pcie->ports, *pos);
>   }
> @@ -2587,16 +2587,16 @@ static int tegra_pcie_ports_seq_show(struct seq_file *s, void *v)
>   	seq_printf(s, "%2u     ", port->index);
>   
>   	if (up)
> -		seq_printf(s, "up");
> +		seq_puts(s, "up");
>   
>   	if (active) {
>   		if (up)
> -			seq_printf(s, ", ");
> +			seq_puts(s, ", ");
>   
> -		seq_printf(s, "active");
> +		seq_puts(s, "active");
>   	}
>   
> -	seq_printf(s, "\n");
> +	seq_puts(s, "\n");
>   	return 0;
>   }
>   
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/3] PCI: tegra: make const array err_msg static
  2021-05-04 17:18 ` [PATCH 3/3] PCI: tegra: make const array err_msg static Christophe JAILLET
@ 2021-07-05 17:01   ` Vidya Sagar
  2021-07-05 22:31   ` Krzysztof Wilczyński
  1 sibling, 0 replies; 11+ messages in thread
From: Vidya Sagar @ 2021-07-05 17:01 UTC (permalink / raw)
  To: Christophe JAILLET, thierry.reding, lorenzo.pieralisi, robh,
	bhelgaas, jonathanh
  Cc: linux-tegra, linux-pci, linux-kernel, kernel-janitors

Reviewed-by: Vidya Sagar <vidyas@nvidia.com>

On 5/4/2021 10:48 PM, Christophe JAILLET wrote:
> Don't populate the array err_msg on the stack but instead make it
> static. Makes the object code smaller by 64 bytes.
> 
> While at it, add a missing const, as reported by checkpatch.
> 
> Compiled with gcc 11.0.1
> 
> Before:
> $ size drivers/pci/controller/pci-tegra.o
>     text	   data	    bss	    dec	    hex	filename
>    25623	   2844	     32	  28499	   6f53	drivers/pci/controller/pci-tegra.o
> 
> After:
> $ size drivers/pci/controller/pci-tegra.o
>     text	   data	    bss	    dec	    hex	filename
>    25559	   2844	     32	  28435	   6f13	drivers/pci/controller/pci-tegra.o
> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
>   drivers/pci/controller/pci-tegra.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c
> index fe8e21ce3e3d..b1250b15d290 100644
> --- a/drivers/pci/controller/pci-tegra.c
> +++ b/drivers/pci/controller/pci-tegra.c
> @@ -764,7 +764,7 @@ static int tegra_pcie_map_irq(const struct pci_dev *pdev, u8 slot, u8 pin)
>   
>   static irqreturn_t tegra_pcie_isr(int irq, void *arg)
>   {
> -	const char *err_msg[] = {
> +	static const char * const err_msg[] = {
>   		"Unknown",
>   		"AXI slave error",
>   		"AXI decode error",
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/3] PCI: tegra: make const array err_msg static
  2021-05-04 17:18 ` [PATCH 3/3] PCI: tegra: make const array err_msg static Christophe JAILLET
  2021-07-05 17:01   ` Vidya Sagar
@ 2021-07-05 22:31   ` Krzysztof Wilczyński
  2021-07-07 18:24     ` Christophe JAILLET
  1 sibling, 1 reply; 11+ messages in thread
From: Krzysztof Wilczyński @ 2021-07-05 22:31 UTC (permalink / raw)
  To: Christophe JAILLET
  Cc: thierry.reding, lorenzo.pieralisi, robh, bhelgaas, jonathanh,
	linux-tegra, linux-pci, linux-kernel, kernel-janitors

Hi Christophe,

Thank you for sending the patches over and taking care about these!

I was wondering whether you will be willing to send a v2 of this series
that would include fixes to everything the checkpatch.pl script reports
against this driver?  There aren't a lot of things to fix, thus the idea
to squash everything at once.  These warnings would be as follows
(excluding the ones you taken care of in this series):

  drivers/pci/controller/pci-tegra.c:1661: WARNING: please, no space before tabs
  drivers/pci/controller/pci-tegra.c:1890: WARNING: quoted string split across lines
  drivers/pci/controller/pci-tegra.c:1891: WARNING: quoted string split across lines
  drivers/pci/controller/pci-tegra.c:2619: WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using octal permissions '0444'.

These should be trivial to fix.  The two pertaining to "quoted string
split across lines" would be something that we might or might not decide
to do anything about this - technically, as per the Linux kernel coding
style [1], we ought to fix this... but, this particular case is not
a terrible example, so I will leave this at your discretion.

What do you think?

Also, don't worry if you don't have the time or otherwise, as these are
trivial things and it would only be a bonus to take care of them.

1. https://www.kernel.org/doc/html/v4.10/process/coding-style.html#breaking-long-lines-and-strings

	Krzysztof

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/3] PCI: tegra: make const array err_msg static
  2021-07-05 22:31   ` Krzysztof Wilczyński
@ 2021-07-07 18:24     ` Christophe JAILLET
  2021-07-07 19:52       ` Krzysztof Wilczyński
  0 siblings, 1 reply; 11+ messages in thread
From: Christophe JAILLET @ 2021-07-07 18:24 UTC (permalink / raw)
  To: Krzysztof Wilczyński
  Cc: thierry.reding, lorenzo.pieralisi, robh, bhelgaas, jonathanh,
	linux-tegra, linux-pci, linux-kernel, kernel-janitors

Le 06/07/2021 à 00:31, Krzysztof Wilczyński a écrit :
> Hi Christophe,
> 
> Thank you for sending the patches over and taking care about these!
> 
> I was wondering whether you will be willing to send a v2 of this series
> that would include fixes to everything the checkpatch.pl script reports
> against this driver?  There aren't a lot of things to fix, thus the idea
> to squash everything at once.  These warnings would be as follows
> (excluding the ones you taken care of in this series):
> 
>    drivers/pci/controller/pci-tegra.c:1661: WARNING: please, no space before tabs
>    drivers/pci/controller/pci-tegra.c:1890: WARNING: quoted string split across lines
>    drivers/pci/controller/pci-tegra.c:1891: WARNING: quoted string split across lines
>    drivers/pci/controller/pci-tegra.c:2619: WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using octal permissions '0444'.
> 
> These should be trivial to fix.  The two pertaining to "quoted string
> split across lines" would be something that we might or might not decide
> to do anything about this - technically, as per the Linux kernel coding
> style [1], we ought to fix this... but, this particular case is not
> a terrible example, so I will leave this at your discretion.
> 
> What do you think?

Hi,
I don't think it worth it.

Even for patch 2/3 about 'seq_printf' --> 'seq_puts' conversion, I'm not 
fully convinced myself that is useful.

Too trivial clean-ups only mess-up 'git blame' for no real added value.

If you want these clean-ups, I can send a patch for it, but checkpatch 
output need sometimes to be ignored on files already in the tree. At 
least, this is my point of view.

CJ



> Also, don't worry if you don't have the time or otherwise, as these are
> trivial things and it would only be a bonus to take care of them.
> 
> 1. https://www.kernel.org/doc/html/v4.10/process/coding-style.html#breaking-long-lines-and-strings
> 
> 	Krzysztof
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/3] PCI: tegra: make const array err_msg static
  2021-07-07 18:24     ` Christophe JAILLET
@ 2021-07-07 19:52       ` Krzysztof Wilczyński
  0 siblings, 0 replies; 11+ messages in thread
From: Krzysztof Wilczyński @ 2021-07-07 19:52 UTC (permalink / raw)
  To: Christophe JAILLET
  Cc: thierry.reding, lorenzo.pieralisi, robh, bhelgaas, jonathanh,
	linux-tegra, linux-pci, linux-kernel, kernel-janitors

Hi Christophe,

[...]
> > These should be trivial to fix.  The two pertaining to "quoted string
> > split across lines" would be something that we might or might not decide
> > to do anything about this - technically, as per the Linux kernel coding
> > style [1], we ought to fix this... but, this particular case is not
> > a terrible example, so I will leave this at your discretion.
> > 
> > What do you think?
> 
> Hi,
> I don't think it worth it.
> 
> Even for patch 2/3 about 'seq_printf' --> 'seq_puts' conversion, I'm not
> fully convinced myself that is useful.

I personally believe it's a good change.

For a literal string without any formatting using the seq_printf() is
much more involved for no reason, but aside of this small performance
improvement, it also has some value in demonstrating the correct usage
patterns - people spent more time reading kernel code and looking at how
to do things and use things to base their work on, so setting some
example is not a bad idea.

Albeit, it's a matter of point of view too, I suppose.

> Too trivial clean-ups only mess-up 'git blame' for no real added value.

Yes, there is a fine line with these.

> If you want these clean-ups, I can send a patch for it, but checkpatch
> output need sometimes to be ignored on files already in the tree. At least,
> this is my point of view.

No worries!  Thank you for giving it some thought!  I appreciate it. :)

	Krzysztof

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] PCI: tegra: Fix OF node reference leak
  2021-05-04 17:17 [PATCH 1/3] PCI: tegra: Fix OF node reference leak Christophe JAILLET
                   ` (3 preceding siblings ...)
  2021-07-05 17:00 ` Vidya Sagar
@ 2021-08-05 10:43 ` Lorenzo Pieralisi
  4 siblings, 0 replies; 11+ messages in thread
From: Lorenzo Pieralisi @ 2021-08-05 10:43 UTC (permalink / raw)
  To: bhelgaas, Christophe JAILLET, jonathanh, thierry.reding, robh
  Cc: Lorenzo Pieralisi, linux-tegra, linux-pci, kernel-janitors, linux-kernel

On Tue, 4 May 2021 19:17:42 +0200, Christophe JAILLET wrote:
> Commit 9e38e690ace3 ("PCI: tegra: Fix OF node reference leak") has fixed
> some node reference leaks in this function but missed some of them.
> 
> In fact, having 'port' referenced in the 'rp' structure is not enough to
> prevent the leak, until 'rp' is actually added in the 'pcie->ports' list.
> 
> Add the missing 'goto err_node_put' accordingly.

Applied to pci/tegra, thanks!

[1/3] PCI: tegra: Fix OF node reference leak
      https://git.kernel.org/lpieralisi/pci/c/eff21f5da3
[2/3] PCI: tegra: Use 'seq_puts' instead of 'seq_printf'
      https://git.kernel.org/lpieralisi/pci/c/804b2b6f2a
[3/3] PCI: tegra: make const array err_msg static
      https://git.kernel.org/lpieralisi/pci/c/fd44e8efcc

Thanks,
Lorenzo

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-08-05 10:43 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-04 17:17 [PATCH 1/3] PCI: tegra: Fix OF node reference leak Christophe JAILLET
2021-05-04 17:17 ` [PATCH 2/3] PCI: tegra: Use 'seq_puts' instead of 'seq_printf' Christophe JAILLET
2021-07-05 17:01   ` Vidya Sagar
2021-05-04 17:18 ` [PATCH 3/3] PCI: tegra: make const array err_msg static Christophe JAILLET
2021-07-05 17:01   ` Vidya Sagar
2021-07-05 22:31   ` Krzysztof Wilczyński
2021-07-07 18:24     ` Christophe JAILLET
2021-07-07 19:52       ` Krzysztof Wilczyński
2021-06-22 10:42 ` [PATCH 1/3] PCI: tegra: Fix OF node reference leak Lorenzo Pieralisi
2021-07-05 17:00 ` Vidya Sagar
2021-08-05 10:43 ` Lorenzo Pieralisi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).