| 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 |
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>
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>
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 <pierre-louis.bossart@linux.intel.com>
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;
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(-)