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

[Takashi Iwai <tiwai@suse.de>]

On Thu, 24 Mar 2016 03:28:21 +0100,
Hsin-Yu Chao wrote:
> 
> This patch enables UCM to set a file in TLV format to kcontrol by:
> cset-tlv "name='<kcontrol-name>' <path-to-file>"
> This new 'cset-tlv' command will be used to write audio DSP to
> specific alsa control, where the driver expectes a file in TLV
> format.
> The TLV file to set to kcontrol will be checked first by file size
> not larger than 16 MB, and then examine if the length field reports
> correct number of bytes in the TLV file.
> 
> Signed-off-by: Hsin-Yu Chao <hychao@chromium.org>
> ---
>  src/ucm/main.c      | 91 +++++++++++++++++++++++++++++++++++++++++++++--------
>  src/ucm/parser.c    | 10 ++++++
>  src/ucm/ucm_local.h |  1 +
>  3 files changed, 89 insertions(+), 13 deletions(-)
> 
> diff --git a/src/ucm/main.c b/src/ucm/main.c
> index 7e44603..d0929a2 100644
> --- a/src/ucm/main.c
> +++ b/src/ucm/main.c
> @@ -161,6 +161,54 @@ static int open_ctl(snd_use_case_mgr_t *uc_mgr,
>  	return 0;
>  }
>  
> +static int read_tlv_file(char **res,
> +			 const char *filepath)
> +{
> +	int err = 0;
> +	int fd;
> +	struct stat st;
> +	size_t sz;
> +	ssize_t sz_read;
> +	unsigned int *tlv;
> +
> +	fd = open(filepath, O_RDONLY);
> +	if (fd < 0) {
> +		err = -errno;
> +		return err;
> +	}
> +	if (stat(filepath, &st) == -1) {

Better to use fstat() with the opened fd.


> +		err = -errno;
> +		goto __fail;
> +	}
> +	sz = st.st_size;
> +	if (sz > 16 * 1024 * 1024 || sz < 8) {
> +		uc_error("File size should be less than 16 MB");
> +		goto __fail;

The error code is missing.


> +	}
> +	*res = malloc(sz);
> +	if (res == NULL) {
> +		err = -ENOMEM;
> +		goto __fail;
> +	}
> +	sz_read = read(fd, *res, sz);
> +	if (sz_read < 0 || (size_t)sz_read != sz) {
> +		err = -errno;

The size shortage won't set the errno.  You'd need to give the error
code explicitly, or repeat the read.


> +		free(*res);
> +		*res = NULL;
> +	}
> +	/* Check if the tlv file specifies valid size. */
> +	tlv = (unsigned int *)(*res);
> +	if (tlv[1] + 2 * sizeof(unsigned int) != sz) {
> +		uc_error("Invalid tlv size");
> +		free(*res);
> +		*res = NULL;

The missing error code.


thanks,

Takashi