Re: [PATCH v4] ucm: add cset-tlv

[Vinod Koul <vinod.koul@intel.com>]

On Wed, Apr 13, 2016 at 02:26:57PM +0800, Hsin-Yu Chao wrote:
> +	sz_read = read(fd, *res, sz);
> +	if (sz_read < 0 || (size_t)sz_read != sz) {
> +		err = -EIO;
> +		free(*res);
> +		*res = NULL;
> +	}
> +	/* Check if the tlv file specifies valid size. */
> +	tlv = (unsigned int *)(*res);
> +	if (tlv[1] + 2 * sizeof(unsigned int) != sz) {

why not use snd_ctl_tlv(), my assumption is that you are expecting a type,
length and values in the buffer

> +		uc_error("Invalid tlv size");

wouldn't it help to print the size for error

> +		err = -EINVAL;
> +		free(*res);
> +		*res = NULL;
> +	}
> +
> +      __fail:

I dont know about syntax expected in UCM but this label could be left
justfied :)

> +	close(fd);
> +	return err;
> +}
> +
>  static int binary_file_parse(snd_ctl_elem_value_t *dst,
>  			      snd_ctl_elem_info_t *info,
>  			      const char *filepath)
> @@ -226,6 +276,7 @@ static int execute_cset(snd_ctl_t *ctl, const char *cset, unsigned int type)
>  	snd_ctl_elem_id_t *id;
>  	snd_ctl_elem_value_t *value;
>  	snd_ctl_elem_info_t *info;
> +	char *res = NULL;
>  
>  	snd_ctl_elem_id_malloc(&id);
>  	snd_ctl_elem_value_malloc(&value);
> @@ -241,23 +292,36 @@ static int execute_cset(snd_ctl_t *ctl, const char *cset, unsigned int type)
>  		err = -EINVAL;
>  		goto __fail;
>  	}
> -	snd_ctl_elem_value_set_id(value, id);
>  	snd_ctl_elem_info_set_id(info, id);
> -	err = snd_ctl_elem_read(ctl, value);
> -	if (err < 0)
> -		goto __fail;
>  	err = snd_ctl_elem_info(ctl, info);
>  	if (err < 0)
>  		goto __fail;
> -	if (type == SEQUENCE_ELEMENT_TYPE_CSET_BIN_FILE)
> -		err = binary_file_parse(value, info, pos);
> -	else
> -		err = snd_ctl_ascii_value_parse(ctl, value, info, pos);
> -	if (err < 0)
> -		goto __fail;
> -	err = snd_ctl_elem_write(ctl, value);
> -	if (err < 0)
> -		goto __fail;
> +	if (type == SEQUENCE_ELEMENT_TYPE_CSET_TLV) {
> +		if (!snd_ctl_elem_info_is_tlv_writable(info)) {
> +			err = -EINVAL;
> +			goto __fail;
> +		}
> +		err = read_tlv_file(&res, pos);
> +		if (err < 0)
> +			goto __fail;
> +		err = snd_ctl_elem_tlv_write(ctl, id, (unsigned int *)res);

if tlv_write expects unsigned int then wouldn't it be better to use
read_tlv_file() with unsigned int.

-- 
~Vinod