[Accel-config] Re: [PATCH] accel-config: Fix a read buffer length bug

On 2/2/2022 3:26 PM, Dave Jiang wrote:
> 
> On 2/2/2022 3:50 PM, Ramesh Thomas wrote:
>> Generic function used to read a string attribute was using a buffer of
>> max length 64. Attributes like op_cap exceeds that size. Change read
>> buffer length to 128. Also removed a redundant memory allocation.
> 
> For sysfs attributes, max buffer size is 4k (a page). Probably should 
> make all reads from sysfs to be that size?

Yes, it should match the driver buffer size.

> 
> 
>> Signed-off-by: Ramesh Thomas <ramesh.thomas(a)intel.com>
>> ---
>>   accfg/lib/libaccfg.c | 14 ++------------
>>   1 file changed, 2 insertions(+), 12 deletions(-)
>>
>> diff --git a/accfg/lib/libaccfg.c b/accfg/lib/libaccfg.c
>> index 9cdd8f8..f20706c 100644
>> --- a/accfg/lib/libaccfg.c
>> +++ b/accfg/lib/libaccfg.c
>> @@ -247,13 +247,13 @@ static uint64_t accfg_get_param_unsigned_llong(
>>   static char *accfg_get_param_str(struct accfg_ctx *ctx, int dfd, 
>> char *name)
>>   {
>>       int fd = openat(dfd, name, O_RDONLY);
>> -    char buf[MAX_PARAM_LEN + 1];
>> +    char buf[MAX_BUF_LEN + 1];
>>       int n;
>>       if (fd == -1)
>>           return NULL;
>> -    n = read(fd, buf, MAX_PARAM_LEN);
>> +    n = read(fd, buf, MAX_BUF_LEN);
>>       close(fd);
>>       if (n <= 0)
>>           return NULL;
>> @@ -618,21 +618,13 @@ static void *add_device(void *parent, int id, 
>> const char *ctl_base,
>>   {
>>       struct accfg_ctx *ctx = parent;
>>       struct accfg_device *device;
>> -    char *path;
>>       int dfd;
>>       int rc;
>>       char *p;
>> -    path = calloc(1, strlen(ctl_base) + MAX_PARAM_LEN);
>> -    if (!path) {
>> -        err(ctx, "%s: allocation of path failed\n", __func__);
>> -        return NULL;
>> -    }
>> -
>>       dfd = open(ctl_base, O_PATH);
>>       if (dfd == -1) {
>>           err(ctx, "%s open failed: %s\n", __func__, strerror(errno));
>> -        free(path);
>>           return NULL;
>>       }
>> @@ -720,7 +712,6 @@ static void *add_device(void *parent, int id, 
>> const char *ctl_base,
>>           goto err_dev_path;
>>       list_add_tail(&ctx->devices, &device->list);
>> -    free(path);
>>       return device;
>> @@ -730,7 +721,6 @@ err_read:
>>       free(device->mdev_path);
>>       free(device);
>>   err_device:
>> -    free(path);
>>       return NULL;
>>   }