[PATCH v2 0/3] Remove support for SNDRV_CTL_ELEM_ACCESS_USER & minor fix

[PATCH v2 0/3] Remove support for SNDRV_CTL_ELEM_ACCESS_USER & minor fix

[mengdong.lin]

From: Mengdong Lin <mengdong.lin@linux.intel.com>

There is no ABI change in this series.

Remove support for control flag SNDRV_CTL_ELEM_ACCESS_USER as Takashi
Sakamoto suggested. Also fix 2 compiler warnings and add check before
string cpy.

History:
v2: Revise commit messages.

Mengdong Lin (3):
  topology: Remove support for control flag SNDRV_CTL_ELEM_ACCESS_USER
  topology: Add ATTRIBUTE_UNUSED for unused parameters to fix compiler
    warning
  topology: Check address and length before string copy

 src/topology/ctl.c        | 1 -
 src/topology/pcm.c        | 8 ++++----
 src/topology/tplg_local.h | 3 +++
 3 files changed, 7 insertions(+), 5 deletions(-)

-- 
2.5.0

[PATCH v2 1/3] topology: Remove support for control flag SNDRV_CTL_ELEM_ACCESS_USER

[mengdong.lin]

From: Mengdong Lin <mengdong.lin@linux.intel.com>

This access flag is used to add controls from user space by ioctl.
But topology only configures controls in user space and these controls
will eventually be created by ASoC in kernel, so topology should not
support this flag.

Signed-off-by: Mengdong Lin <mengdong.lin@linux.intel.com>

diff --git a/src/topology/ctl.c b/src/topology/ctl.c
index 592dded..54f8e44 100644
--- a/src/topology/ctl.c
+++ b/src/topology/ctl.c
@@ -41,7 +41,6 @@ static const struct ctl_access_elem ctl_access[] = {
 	{"lock", SNDRV_CTL_ELEM_ACCESS_LOCK},
 	{"owner", SNDRV_CTL_ELEM_ACCESS_OWNER},
 	{"tlv_callback", SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK},
-	{"user", SNDRV_CTL_ELEM_ACCESS_USER},
 };
 
 /* find CTL access strings and conver to values */
-- 
2.5.0

[PATCH v2 2/3] topology: Add ATTRIBUTE_UNUSED for unused parameters to fix compiler warning

[mengdong.lin]

From: Mengdong Lin <mengdong.lin@linux.intel.com>

This commit adds ATTRIBUTE_UNUSED to the unused parameter 'tplg' for
function tplg_parse_streams() and tplg_parse_fe_dai(). These two functions
need to keep 'tplg' as ops for tplg_parse_compound().

The compiler warnings below are fixed:

pcm.c: In function 'tplg_parse_streams':
pcm.c:262:43: warning: unused parameter 'tplg' [-Wunused-parameter]
 static int tplg_parse_streams(snd_tplg_t *tplg, snd_config_t *cfg,
                                           ^
pcm.c: In function 'tplg_parse_fe_dai':
pcm.c:324:42: warning: unused parameter 'tplg' [-Wunused-parameter]
 static int tplg_parse_fe_dai(snd_tplg_t *tplg, snd_config_t *cfg,

Signed-off-by: Mengdong Lin <mengdong.lin@linux.intel.com>

diff --git a/src/topology/pcm.c b/src/topology/pcm.c
index 281e6ef..0a90cb9 100644
--- a/src/topology/pcm.c
+++ b/src/topology/pcm.c
@@ -259,8 +259,8 @@ int tplg_parse_stream_caps(snd_tplg_t *tplg,
 }
 
 /* Parse the caps and config of a pcm stream */
-static int tplg_parse_streams(snd_tplg_t *tplg, snd_config_t *cfg,
-	void *private)
+static int tplg_parse_streams(snd_tplg_t *tplg ATTRIBUTE_UNUSED,
+			      snd_config_t *cfg, void *private)
 {
 	snd_config_iterator_t i, next;
 	snd_config_t *n;
@@ -321,8 +321,8 @@ static int tplg_parse_streams(snd_tplg_t *tplg, snd_config_t *cfg,
 }
 
 /* Parse name and id of a front-end DAI (ie. cpu dai of a FE DAI link) */
-static int tplg_parse_fe_dai(snd_tplg_t *tplg, snd_config_t *cfg,
-	void *private)
+static int tplg_parse_fe_dai(snd_tplg_t *tplg ATTRIBUTE_UNUSED,
+			     snd_config_t *cfg, void *private)
 {
 	struct tplg_elem *elem = private;
 	struct snd_soc_tplg_pcm *pcm = elem->pcm;
-- 
2.5.0

[PATCH v2 3/3] topology: Check address and length before string copy

[mengdong.lin]

From: Mengdong Lin <mengdong.lin@linux.intel.com>

elem_copy_text() is widely used for string copy in topology. Because some
name fields are not mandatory for users, sometimes the source can be
invalid and we should not do the copy. So we add check here.

Signed-off-by: Mengdong Lin <mengdong.lin@linux.intel.com>

diff --git a/src/topology/tplg_local.h b/src/topology/tplg_local.h
index 4d79aa7..cfde4cc 100644
--- a/src/topology/tplg_local.h
+++ b/src/topology/tplg_local.h
@@ -253,6 +253,9 @@ struct tplg_elem* tplg_elem_new_common(snd_tplg_t *tplg,
 
 static inline void elem_copy_text(char *dest, const char *src, int len)
 {
+	if (!dest || !src || !len)
+		return;
+
 	strncpy(dest, src, len);
 	dest[len - 1] = 0;
 }
-- 
2.5.0

Re: [PATCH v2 0/3] Remove support for SNDRV_CTL_ELEM_ACCESS_USER & minor fix

[Takashi Sakamoto]

Hi,

On Jul 20 2016 10:52, mengdong.lin@linux.intel.com wrote:
> From: Mengdong Lin <mengdong.lin@linux.intel.com>
>
> There is no ABI change in this series.
>
> Remove support for control flag SNDRV_CTL_ELEM_ACCESS_USER as Takashi
> Sakamoto suggested. Also fix 2 compiler warnings and add check before
> string cpy.
>
> History:
> v2: Revise commit messages.
>
> Mengdong Lin (3):
>    topology: Remove support for control flag SNDRV_CTL_ELEM_ACCESS_USER
>    topology: Add ATTRIBUTE_UNUSED for unused parameters to fix compiler
>      warning
>    topology: Check address and length before string copy
>
>   src/topology/ctl.c        | 1 -
>   src/topology/pcm.c        | 8 ++++----
>   src/topology/tplg_local.h | 3 +++
>   3 files changed, 7 insertions(+), 5 deletions(-)

Reviewed-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>

But I believe that we can seek better ways for the purpose of third 
patch. For example, changing prototype of the function to return error 
code might help callers to handle errors appropriately.

Anyway, thanks for the first patch.


Regards

Takashi Sakamoto

Re: [PATCH v2 0/3] Remove support for SNDRV_CTL_ELEM_ACCESS_USER & minor fix

[Mengdong Lin]

On 07/20/2016 12:29 PM, Takashi Sakamoto wrote:
> Hi,
>
> On Jul 20 2016 10:52, mengdong.lin@linux.intel.com wrote:
>> From: Mengdong Lin <mengdong.lin@linux.intel.com>
>>
>> There is no ABI change in this series.
>>
>> Remove support for control flag SNDRV_CTL_ELEM_ACCESS_USER as Takashi
>> Sakamoto suggested. Also fix 2 compiler warnings and add check before
>> string cpy.
>>
>> History:
>> v2: Revise commit messages.
>>
>> Mengdong Lin (3):
>>    topology: Remove support for control flag SNDRV_CTL_ELEM_ACCESS_USER
>>    topology: Add ATTRIBUTE_UNUSED for unused parameters to fix compiler
>>      warning
>>    topology: Check address and length before string copy
>>
>>   src/topology/ctl.c        | 1 -
>>   src/topology/pcm.c        | 8 ++++----
>>   src/topology/tplg_local.h | 3 +++
>>   3 files changed, 7 insertions(+), 5 deletions(-)
>
> Reviewed-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
>
> But I believe that we can seek better ways for the purpose of third
> patch. For example, changing prototype of the function to return error
> code might help callers to handle errors appropriately.
>
> Anyway, thanks for the first patch.
>

The 3rd patch is to reduce checking on some optional fields elsewhere. 
We could drop this patch if we add check before calling elem_copy_text() 
and it will bring more code.

Thanks
Mengdong

Re: [PATCH v2 0/3] Remove support for SNDRV_CTL_ELEM_ACCESS_USER & minor fix

[Takashi Iwai]

On Wed, 20 Jul 2016 03:52:23 +0200,
mengdong.lin@linux.intel.com wrote:
> 
> From: Mengdong Lin <mengdong.lin@linux.intel.com>
> 
> There is no ABI change in this series.
> 
> Remove support for control flag SNDRV_CTL_ELEM_ACCESS_USER as Takashi
> Sakamoto suggested. Also fix 2 compiler warnings and add check before
> string cpy.
> 
> History:
> v2: Revise commit messages.
> 
> Mengdong Lin (3):
>   topology: Remove support for control flag SNDRV_CTL_ELEM_ACCESS_USER
>   topology: Add ATTRIBUTE_UNUSED for unused parameters to fix compiler
>     warning
>   topology: Check address and length before string copy

Applied all three patches now.  Thanks.


Takashi