[alsa-devel] [PATCH v3] ucm: add cset-tlv

[Hsin-yu Chao]

Hi Takashi,
Really sorry for missed your reply for so long due to a problem of my
inbox filter...
I will upload a new patch to address your comments.

Thanks,
Hsin-yu

On Thu, Mar 24, 2016 at 9:19 PM, Takashi Iwai <tiwai at suse.de> wrote:
> 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 at 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