* [Accel-config] Re: [PATCH] accel-config: Fix a read buffer length bug
@ 2022-02-02 23:26 Dave Jiang
0 siblings, 0 replies; 3+ messages in thread
From: Dave Jiang @ 2022-02-02 23:26 UTC (permalink / raw)
To: accel-config
[-- Attachment #1: Type: text/plain, Size: 2131 bytes --]
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?
> 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;
> }
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* [Accel-config] Re: [PATCH] accel-config: Fix a read buffer length bug
@ 2022-02-03 16:14 Dave Jiang
0 siblings, 0 replies; 3+ messages in thread
From: Dave Jiang @ 2022-02-03 16:14 UTC (permalink / raw)
To: accel-config
[-- Attachment #1: Type: text/plain, Size: 3987 bytes --]
On 2/2/2022 6:43 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 4k. Also removed a redundant memory allocation.
>
> Signed-off-by: Ramesh Thomas <ramesh.thomas(a)intel.com>
Reviewed-by: Dave Jiang <dave.jiang(a)intel.com>
> ---
> accfg/lib/libaccfg.c | 24 +++++++-----------------
> accfg/libaccel_config.h | 2 +-
> 2 files changed, 8 insertions(+), 18 deletions(-)
>
> diff --git a/accfg/lib/libaccfg.c b/accfg/lib/libaccfg.c
> index 9cdd8f8..e6c6055 100644
> --- a/accfg/lib/libaccfg.c
> +++ b/accfg/lib/libaccfg.c
> @@ -197,7 +197,7 @@ static int accfg_set_param(struct accfg_ctx *ctx, int dfd, char *name,
> static long accfg_get_param_long(struct accfg_ctx *ctx, int dfd, char *name)
> {
> int fd = openat(dfd, name, O_RDONLY);
> - char buf[MAX_PARAM_LEN + 1];
> + char buf[MAX_PARAM_LEN];
> int n;
> const char *p;
>
> @@ -210,7 +210,7 @@ static long accfg_get_param_long(struct accfg_ctx *ctx, int dfd, char *name)
> if (fd == -1)
> return -errno;
>
> - n = read(fd, buf, MAX_PARAM_LEN);
> + n = read(fd, buf, MAX_PARAM_LEN - 1);
> close(fd);
> if (n <= 0)
> return -ENXIO;
> @@ -226,13 +226,13 @@ static uint64_t accfg_get_param_unsigned_llong(
> struct accfg_ctx *ctx, int dfd, char *name)
> {
> int fd = openat(dfd, name, O_RDONLY);
> - char buf[MAX_PARAM_LEN + 1];
> + char buf[MAX_PARAM_LEN];
> int n;
>
> if (fd == -1)
> return -errno;
>
> - n = read(fd, buf, MAX_PARAM_LEN);
> + n = read(fd, buf, MAX_PARAM_LEN - 1);
> close(fd);
> if (n <= 0)
> return -ENXIO;
> @@ -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_PARAM_LEN];
> int n;
>
> if (fd == -1)
> return NULL;
>
> - n = read(fd, buf, MAX_PARAM_LEN);
> + n = read(fd, buf, MAX_PARAM_LEN - 1);
> 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;
> }
>
> @@ -702,7 +694,7 @@ static void *add_device(void *parent, int id, const char *ctl_base,
> device->mdev_path = p;
>
> device->device_buf = calloc(1, strlen(device->device_path) +
> - MAX_PARAM_LEN);
> + MAX_BUF_LEN);
> if (!device->device_buf) {
> err(ctx, "allocation of device buffer failed\n");
> goto err_read;
> @@ -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;
> }
>
> diff --git a/accfg/libaccel_config.h b/accfg/libaccel_config.h
> index 8e7a74e..9e952a7 100644
> --- a/accfg/libaccel_config.h
> +++ b/accfg/libaccel_config.h
> @@ -22,7 +22,7 @@ extern "C" {
>
> #define MAX_DEV_LEN 64
> #define MAX_BUF_LEN 128
> -#define MAX_PARAM_LEN 64
> +#define MAX_PARAM_LEN 4096
> #define TRAFFIC_CLASS_LIMIT 8
> #define WQ_PRIORITY_LIMIT 15
> #define UUID_ZERO "00000000-0000-0000-0000-000000000000"
^ permalink raw reply [flat|nested] 3+ messages in thread
* [Accel-config] Re: [PATCH] accel-config: Fix a read buffer length bug
@ 2022-02-03 1:02 Ramesh Thomas
0 siblings, 0 replies; 3+ messages in thread
From: Ramesh Thomas @ 2022-02-03 1:02 UTC (permalink / raw)
To: accel-config
[-- Attachment #1: Type: text/plain, Size: 2587 bytes --]
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;
>> }
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-02-03 16:14 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-02 23:26 [Accel-config] Re: [PATCH] accel-config: Fix a read buffer length bug Dave Jiang
2022-02-03 1:02 Ramesh Thomas
2022-02-03 16:14 Dave Jiang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.