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 <[email protected]>
---
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;
--
2.45.0
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 <[email protected]>
---
Compile tested-only.
---
sound/soc/intel/avs/topology.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/intel/avs/topology.c b/sound/soc/intel/avs/topology.c
index 02bae207f6ec..35381a835c93 100644
--- a/sound/soc/intel/avs/topology.c
+++ b/sound/soc/intel/avs/topology.c
@@ -1889,7 +1889,7 @@ avs_control_load(struct snd_soc_component *comp, int index, struct snd_kcontrol_
return 0;
}
-static struct snd_soc_tplg_ops avs_tplg_ops = {
+static const struct snd_soc_tplg_ops avs_tplg_ops = {
.io_ops = avs_control_ops,
.io_ops_count = ARRAY_SIZE(avs_control_ops),
.control_load = avs_control_load,
--
2.45.0
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 <[email protected]>
> ---
> 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 <[email protected]>
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 <[email protected]>
> ---
Reviewed-by: Amadeusz Sławiński <[email protected]>
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.
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
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.
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 <[email protected]>