[alsa-devel] [PATCH] topology: Fix the missing referenced elem ptr when merging private data

Takashi Sakamoto o-takashi at sakamocchi.jp
Thu Jul 21 23:16:03 CEST 2016


On Jul 22 2016 05:37, Takashi Sakamoto wrote:
> Hi,
> 
> On Jul 21 2016 16:00, mengdong.lin at linux.intel.com wrote:
>> From: Mengdong Lin <mengdong.lin at linux.intel.com>
>>
>> tplg_copy_data() should set the valid referenced data element pointer.
>> The caller will double check this pointer for all kinds of references,
>> including TLV, controls, text and data.
>>
>> Also fix an inaccurate dmesg.
>>
>> Signed-off-by: Mengdong Lin <mengdong.lin at linux.intel.com>
>>
>> diff --git a/src/topology/dapm.c b/src/topology/dapm.c
>> index 4d343b2..e3c90d8 100644
>> --- a/src/topology/dapm.c
>> +++ b/src/topology/dapm.c
>> @@ -201,7 +201,7 @@ static int tplg_build_widget(snd_tplg_t *tplg,
>>  		}
>>  
>>  		if (!ref->elem) {
>> -			SNDERR("error: cannot find control '%s'"
>> +			SNDERR("error: cannot find '%s'"
>>  				" referenced by widget '%s'\n",
>>  				ref->id, elem->id);
>>  			return -EINVAL;
> 
> It's better to split to two patches for the different aims. Accumulating
> patches to log history is preferable, as long as you have concrete wills
> to improve codes, I believe.
> 
>> diff --git a/src/topology/data.c b/src/topology/data.c
>> index 768fc27..397f6a5 100644
>> --- a/src/topology/data.c
>> +++ b/src/topology/data.c
>> @@ -1050,6 +1050,7 @@ int tplg_copy_data(snd_tplg_t *tplg, struct tplg_elem *elem,
>>  		" element '%s'\n", ref->id, elem->id);
>>  		return -EINVAL;
>>  	}
>> +	ref->elem = ref_elem;
>>  
>>  	tplg_dbg("Data '%s' used by '%s'\n", ref->id, elem->id);
>>  	/* overlook empty private data */
> 
> I wonder why the assignment is done in this line. This function still
> includes some codes to return errors. In this case, it's better to code
> the assignment in last part of the function.
> 
> This is a public API. It's better not to touch application's objects in
> invalid cases.

Oops. I overlooked. It's in src/topology/tplg_local.h and not a public
API... Anyway, it's better to consider about the error cases for
consistency of object state.


Regards

Takashi Sakamoto


More information about the Alsa-devel mailing list