On Tue, Mar 22, 2016 at 7:48 PM, Takashi Iwai tiwai@suse.de wrote:
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@chromium.org
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.
Thanks for the review. I think by extracting the length attribute (i.e tlv[1]) and compare it with the file size is sufficient here, since there is no way to figure out the endianness of the binary file passed in. Also I agree that additional check for crazy large or small file is necessary. According to the dsp files I test with, I'll set the tlv file size limit to between 8 bytes and 1MB. Will upload a v2 patch shortly, thanks!
Hsin-yu
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@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel