diff mbox series

[1/6] ASoC: topology: Constify an argument of snd_soc_tplg_component_load()

Message ID f2f983e791d7f941a95556bb147f426a345d84d4.1715526069.git.christophe.jaillet@wanadoo.fr
State Accepted
Commit 734447685ecc7c6328e40cb1bd4aaeeac03c1413
Headers show
Series [1/6] ASoC: topology: Constify an argument of snd_soc_tplg_component_load() | expand

Commit Message

Christophe JAILLET May 13, 2024, 5:37 p.m. UTC
snd_soc_tplg_component_load() does not modify its "*ops" argument. It
only read some values and stores it in "soc_tplg.ops".

This argument and the ops field in "struct soc_tplg" can be made const.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
 include/sound/soc-topology.h | 2 +-
 sound/soc/soc-topology.c     | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

Comments

Amadeusz Sławiński May 14, 2024, 7:23 a.m. UTC | #1
On 5/13/2024 7:37 PM, Christophe JAILLET wrote:
> snd_soc_tplg_component_load() does not modify its "*ops" argument. It
> only read some values and stores it in "soc_tplg.ops".
> 
> This argument and the ops field in "struct soc_tplg" can be made const.
> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
>   include/sound/soc-topology.h | 2 +-
>   sound/soc/soc-topology.c     | 4 ++--
>   2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/include/sound/soc-topology.h b/include/sound/soc-topology.h
> index f055c6917f6c..1eedd203ac29 100644
> --- a/include/sound/soc-topology.h
> +++ b/include/sound/soc-topology.h
> @@ -178,7 +178,7 @@ static inline const void *snd_soc_tplg_get_data(struct snd_soc_tplg_hdr *hdr)
>   
>   /* Dynamic Object loading and removal for component drivers */
>   int snd_soc_tplg_component_load(struct snd_soc_component *comp,
> -	struct snd_soc_tplg_ops *ops, const struct firmware *fw);
> +	const struct snd_soc_tplg_ops *ops, const struct firmware *fw);
>   int snd_soc_tplg_component_remove(struct snd_soc_component *comp);
>   
>   /* Binds event handlers to dynamic widgets */
> diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
> index 90ca37e008b3..b00ec01361c2 100644
> --- a/sound/soc/soc-topology.c
> +++ b/sound/soc/soc-topology.c
> @@ -73,7 +73,7 @@ struct soc_tplg {
>   	int bytes_ext_ops_count;
>   
>   	/* optional fw loading callbacks to component drivers */
> -	struct snd_soc_tplg_ops *ops;
> +	const struct snd_soc_tplg_ops *ops;
>   };
>   
>   /* check we dont overflow the data for this control chunk */
> @@ -2334,7 +2334,7 @@ static int soc_tplg_load(struct soc_tplg *tplg)
>   
>   /* load audio component topology from "firmware" file */
>   int snd_soc_tplg_component_load(struct snd_soc_component *comp,
> -	struct snd_soc_tplg_ops *ops, const struct firmware *fw)
> +	const struct snd_soc_tplg_ops *ops, const struct firmware *fw)
>   {
>   	struct soc_tplg tplg;
>   	int ret;

Yes, makes sense to me.

Reviewed-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Amadeusz Sławiński May 14, 2024, 7:24 a.m. UTC | #2
On 5/13/2024 7:37 PM, Christophe JAILLET wrote:
> Constifying "struct snd_soc_tplg_ops" moves some data to a read-only
> section, so increase overall security.
> 
> On a x86_64, with allmodconfig:
> Before:
>     text	   data	    bss	    dec	    hex	filename
>    28046	    794	      0	  28840	   70a8	sound/soc/intel/avs/topology.o
> 
> After:
>     text	   data	    bss	    dec	    hex	filename
>    28206	    614	      0	  28820	   7094	sound/soc/intel/avs/topology.o
> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---

Reviewed-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Mark Brown May 14, 2024, 10:21 a.m. UTC | #3
On Mon, May 13, 2024 at 07:37:20PM +0200, Christophe JAILLET wrote:
> snd_soc_tplg_component_load() does not modify its "*ops" argument. It
> only read some values and stores it in "soc_tplg.ops".

As mentioned in submitting-patches.rst when submitting a patch series
you should supply a cover letter for that patch series which describes
the overall content of the series.  This helps people understand what
they are looking at and how things fit together.
Christophe JAILLET May 18, 2024, 8:34 a.m. UTC | #4
Le 14/05/2024 à 12:21, Mark Brown a écrit :
> On Mon, May 13, 2024 at 07:37:20PM +0200, Christophe JAILLET wrote:
>> snd_soc_tplg_component_load() does not modify its "*ops" argument. It
>> only read some values and stores it in "soc_tplg.ops".
> 
> As mentioned in submitting-patches.rst when submitting a patch series
> you should supply a cover letter for that patch series which describes
> the overall content of the series.  This helps people understand what
> they are looking at and how things fit together.

Ok.
I usually do, but I thought that the subjects were self-explanatory 
enough in this case.

Do you want me to resend?

CJ
Mark Brown May 20, 2024, 1:24 p.m. UTC | #5
On Sat, May 18, 2024 at 10:34:33AM +0200, Christophe JAILLET wrote:
> Le 14/05/2024 à 12:21, Mark Brown a écrit :

> > As mentioned in submitting-patches.rst when submitting a patch series
> > you should supply a cover letter for that patch series which describes
> > the overall content of the series.  This helps people understand what
> > they are looking at and how things fit together.

> Ok.
> I usually do, but I thought that the subjects were self-explanatory enough
> in this case.

> Do you want me to resend?

It's fine this time.
Pierre-Louis Bossart May 20, 2024, 9:31 p.m. UTC | #6
On 5/20/24 08:24, Mark Brown wrote:
> On Sat, May 18, 2024 at 10:34:33AM +0200, Christophe JAILLET wrote:
>> Le 14/05/2024 à 12:21, Mark Brown a écrit :
> 
>>> As mentioned in submitting-patches.rst when submitting a patch series
>>> you should supply a cover letter for that patch series which describes
>>> the overall content of the series.  This helps people understand what
>>> they are looking at and how things fit together.
> 
>> Ok.
>> I usually do, but I thought that the subjects were self-explanatory enough
>> in this case.
> 
>> Do you want me to resend?
> 
> It's fine this time.

no issues with
https://github.com/thesofproject/linux/pull/4993, so

Tested-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
diff mbox series

Patch

diff --git a/include/sound/soc-topology.h b/include/sound/soc-topology.h
index f055c6917f6c..1eedd203ac29 100644
--- a/include/sound/soc-topology.h
+++ b/include/sound/soc-topology.h
@@ -178,7 +178,7 @@  static inline const void *snd_soc_tplg_get_data(struct snd_soc_tplg_hdr *hdr)
 
 /* Dynamic Object loading and removal for component drivers */
 int snd_soc_tplg_component_load(struct snd_soc_component *comp,
-	struct snd_soc_tplg_ops *ops, const struct firmware *fw);
+	const struct snd_soc_tplg_ops *ops, const struct firmware *fw);
 int snd_soc_tplg_component_remove(struct snd_soc_component *comp);
 
 /* Binds event handlers to dynamic widgets */
diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index 90ca37e008b3..b00ec01361c2 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -73,7 +73,7 @@  struct soc_tplg {
 	int bytes_ext_ops_count;
 
 	/* optional fw loading callbacks to component drivers */
-	struct snd_soc_tplg_ops *ops;
+	const struct snd_soc_tplg_ops *ops;
 };
 
 /* check we dont overflow the data for this control chunk */
@@ -2334,7 +2334,7 @@  static int soc_tplg_load(struct soc_tplg *tplg)
 
 /* load audio component topology from "firmware" file */
 int snd_soc_tplg_component_load(struct snd_soc_component *comp,
-	struct snd_soc_tplg_ops *ops, const struct firmware *fw)
+	const struct snd_soc_tplg_ops *ops, const struct firmware *fw)
 {
 	struct soc_tplg tplg;
 	int ret;