Re: [PATCH alsa-lib 8/8] topology: Make buffer for saving dynamic size

[Piotr Maziarz <piotrx.maziarz@linux.intel.com>]

On 2020-06-25 16:31, Pierre-Louis Bossart wrote:
> 
> 
> On 6/25/20 6:03 AM, Piotr Maziarz wrote:
>> Some fields can exceed size limit, e.g. private data has no size
>> restriction. Therefore it needs to be dynamically increased.
>>
>> Signed-off-by: Piotr Maziarz <piotrx.maziarz@linux.intel.com>
>> ---
>>   src/topology/save.c | 36 ++++++++++++++++++++++++++++++------
>>   1 file changed, 30 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/topology/save.c b/src/topology/save.c
>> index 4ecf86c..d6ee8b6 100644
>> --- a/src/topology/save.c
>> +++ b/src/topology/save.c
>> @@ -19,22 +19,43 @@
>>   #include "tplg_local.h"
>>   #define SAVE_ALLOC_SHIFT    (13)    /* 8192 bytes */
>> +#define PRINT_BUF_SIZE        (1024)
>> +#define PRINT_BUF_SIZE_MAX    (1024 * 1024)
>>   int tplg_save_printf(char **dst, const char *pfx, const char *fmt, ...)
>>   {
>>       va_list va;
>> -    char buf[1024], *s;
>> -    size_t n, l, t, pl;
>> +    char *buf, *s;
>> +    size_t n, alloc_size = PRINT_BUF_SIZE, l, t, pl;
>> +    int ret = 0;
>> +
>> +    buf = malloc(alloc_size);
>> +    if (!buf)
>> +        return -ENOMEM;
>>       if (pfx == NULL)
>>           pfx = "";
>> +again:
>>       va_start(va, fmt);
>> -    n = vsnprintf(buf, sizeof(buf), fmt, va);
>> +    n = vsnprintf(buf, alloc_size, fmt, va);
>>       va_end(va);
>> -    if (n >= sizeof(buf))
>> -        return -EOVERFLOW;
>> +    if (n >= PRINT_BUF_SIZE_MAX) {
>> +        ret = -EOVERFLOW;
>> +        goto end;
>> +    }
> 
> what this patch does is change the allocation limit from 1KB to 1MB, but 
> the data still has no size restriction. At what point do we decide to 
> throw an error?
> 
If needed size is bigger than PRINT_BUF_SIZE_MAX there will be an error 
thrown so I don't know what should I change here. Or do you mean that 
private data size in binary should be restricted?