Re: [PATCH v2 6/7] topology: Add support for parsing vendor tuples

[Mengdong Lin <mengdong.lin@linux.intel.com>]

On 03/30/2016 03:35 PM, Takashi Iwai wrote:
> On Wed, 30 Mar 2016 09:11:17 +0200,
> mengdong.lin@linux.intel.com wrote:
>>
>> +		switch (type) {
>> +		case SND_SOC_TPLG_TUPLE_TYPE_UUID:
>> +			len = strlen(value);
>> +			if (len > 16 || len == 0) {
>> +				SNDERR("error: tuple %s: invalid uuid\n", id);
>> +				goto err;
>> +			}
>> +
>> +			memcpy(tuple->uuid, value, 16);
>
> This may still overflow :)
> How about simply using elem_copy_text()?

Sorry for the late reply.

Would you mind me using uuid_parse() here?
It can convert an input UUID string into the binary representation.

An UUID string link "1b4e28ba-2fa1-11d2-883f-b9a761bde3fb" is user 
friendly for the text conf file. But this will add dependency on libuuid.

>
>> +		case SND_SOC_TPLG_TUPLE_TYPE_BYTE:
>> +		case SND_SOC_TPLG_TUPLE_TYPE_SHORT:
>> +		case SND_SOC_TPLG_TUPLE_TYPE_WORD:
>> +			tuple_val = strtol(value, NULL, 0);
>> +			if (tuple_val == LONG_MIN || tuple_val == LONG_MAX
>> +				|| (type == SND_SOC_TPLG_TUPLE_TYPE_WORD
>> +					&& tuple_val > 0xffffffff)
>
> Is the check correct on 32bit architecture?

I'll test on 32 bit machine and will use UINT/USHRT/UCHAR_MAX for range 
here.

>> +				|| (type == SND_SOC_TPLG_TUPLE_TYPE_SHORT
>> +					&& tuple_val > 0xffff)
>> +				|| (type == SND_SOC_TPLG_TUPLE_TYPE_BYTE
>> +					&& tuple_val > 0xff)) {
>
> Also, what about negative values?

I will add check, we don't expect to have negative value here.

Thanks again for your review.

Regards
Mengdong

>
> Takashi
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>