On Tue, 22 Mar 2016 10:10:34 +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.
>
> Signed-off-by: Hsin-Yu Chao <hychao@xxxxxxxxxxxx>
One problem in this approach is that the provided TLV data file isn't
portable. Since we deal TLV as int arrays, it's endian-sensitive.
At least, some endian check would be needed.
Some other nitpicks:
> ---
> src/ucm/main.c | 80 ++++++++++++++++++++++++++++++++++++++++++++---------
> src/ucm/parser.c | 10 +++++++
> src/ucm/ucm_local.h | 1 +
> 3 files changed, 78 insertions(+), 13 deletions(-)
>
> diff --git a/src/ucm/main.c b/src/ucm/main.c
> index 7e44603..a4ccb65 100644
> --- a/src/ucm/main.c
> +++ b/src/ucm/main.c
> @@ -161,6 +161,47 @@ static int open_ctl(snd_use_case_mgr_t *uc_mgr,
> return 0;
> }
>
> +static int tlv_parse(char **res,
> + snd_ctl_elem_info_t *info,
> + const char *filepath)
The function name is somehow confusing. It appears as if it parses
TLV. And what it actually does are two things:
- check snd_ctl_elem_info_is_tlv_writable(),
and
- allocate the whole TLV file.
IMO, snd_ctl_elem_info_is_tlv_writable() check can be in the caller
side, and this function (rename whatever better) can just do allocate
and read the TLV data into a buffer.
> +{
> + int err = 0;
> + int fd;
> + struct stat st;
> + size_t sz;
> + ssize_t sz_read;
> +
> + if (!snd_ctl_elem_info_is_tlv_writable(info)) {
> + err = -EINVAL;
> + return err;
> + }
> + fd = open(filepath, O_RDONLY);
> + if (fd < 0) {
> + err = -errno;
> + return err;
> + }
> + if (stat(filepath, &st) == -1) {
> + err = -errno;
> + goto __fail;
> + }
> + sz = st.st_size;
> + *res = malloc(sz);
What if a crazy large file was passed? A sanity check of the file
size might be good.
thanks,
Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
