[Accel-config] Re: [PATCH v1] accel-config: Fix static code analysis issues

[Accel-config] Re: [PATCH v1] accel-config: Fix static code analysis issues

[Thomas, Ramesh]

[-- Attachment #1: Type: text/plain, Size: 738 bytes --]

On Fri, Dec 04, 2020 at 08:23:53AM, Tony Luck wrote:
> @@ -615,7 +616,6 @@ err_read:
>  free(wq->wq_buf);
>  free(wq->wq_path);
>  err_wq:
> -close(dfd);
>  free(wq);
>  free(path);
>  return NULL;
> 
> Maybe I'm not looking at the latest source ... but it looks like there
> is one path to this code where dfd isn't closed first. Here:
> 
>  640         /* parse uuid only in mdeve wq type */
>  641         if (wq->type == ACCFG_WQT_MDEV) {
>  642                 ret = uuid_entry_add(ctx, wq, dfd);
>  643                 if (ret < 0)
>  644                         goto err_wq;
>  645         }

Yes, that is old code. We removed mdevs from wqs.
Use repo at https://github.com/intel/idxd-config.git pending branch

[Accel-config] Re: [PATCH v1] accel-config: Fix static code analysis issues

[Luck, Tony]

[-- Attachment #1: Type: text/plain, Size: 536 bytes --]

@@ -615,7 +616,6 @@ err_read:
 	free(wq->wq_buf);
 	free(wq->wq_path);
 err_wq:
-	close(dfd);
 	free(wq);
 	free(path);
 	return NULL;

Maybe I'm not looking at the latest source ... but it looks like there
is one path to this code where dfd isn't closed first. Here:

 640         /* parse uuid only in mdeve wq type */
 641         if (wq->type == ACCFG_WQT_MDEV) {
 642                 ret = uuid_entry_add(ctx, wq, dfd);
 643                 if (ret < 0)
 644                         goto err_wq;
 645         }

[Accel-config] Re: [PATCH v1] accel-config: Fix static code analysis issues

[Jiang, Dave]

[-- Attachment #1: Type: text/plain, Size: 3207 bytes --]



> -----Original Message-----
> From: ramesh.thomas(a)intel.com <ramesh.thomas(a)intel.com>
> Sent: Friday, December 4, 2020 4:11 AM
> To: accel-config(a)lists.01.org
> Cc: Luck, Tony <tony.luck(a)intel.com>; Jiang, Dave <dave.jiang(a)intel.com>;
> Thomas, Ramesh <ramesh.thomas(a)intel.com>
> Subject: [PATCH v1] accel-config: Fix static code analysis issues
> 
> From: Ramesh Thomas <ramesh.thomas(a)intel.com>
> 
> Fixed bugs found in static code analysis

For future bisect history, can you paste in the static code analysis complaint and explain what was fixed? Thanks!
 
> 
> Signed-off-by: Ramesh Thomas <ramesh.thomas(a)intel.com>
> ---
>  accfg/lib/libaccfg.c | 10 ++++++----
>  accfg/list.c         |  3 +--
>  2 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/accfg/lib/libaccfg.c b/accfg/lib/libaccfg.c index cd9da42..3b53be3
> 100644
> --- a/accfg/lib/libaccfg.c
> +++ b/accfg/lib/libaccfg.c
> @@ -529,7 +529,7 @@ static void *add_wq(void *parent, int id, const char
> *wq_base,  {
>  	struct accfg_wq *wq;
>  	struct accfg_device *device = parent;
> -	struct accfg_group *group = device->group;
> +	struct accfg_group *group;
>  	struct accfg_ctx *ctx;
>  	char *path;
>  	char *wq_base_string;
> @@ -540,6 +540,7 @@ static void *add_wq(void *parent, int id, const char
> *wq_base,
>  	if (!device)
>  		return NULL;
> 
> +	group = device->group;
>  	ctx = accfg_device_get_ctx(device);
>  	dfd = open(wq_base, O_PATH);
>  	if (dfd < 0)
> @@ -615,7 +616,6 @@ err_read:
>  	free(wq->wq_buf);
>  	free(wq->wq_path);
>  err_wq:
> -	close(dfd);
>  	free(wq);
>  	free(path);
>  	return NULL;
> @@ -715,7 +715,7 @@ static void *add_engine(void *parent, int id, const
> char *engine_base,
>  	struct accfg_engine *engine;
>  	struct accfg_device *device = parent;
>  	struct accfg_ctx *ctx;
> -	struct accfg_group *group = device->group;
> +	struct accfg_group *group;
>  	char *path;
>  	char *engine_base_string;
>  	int dfd;
> @@ -724,6 +724,7 @@ static void *add_engine(void *parent, int id, const
> char *engine_base,
>  	if (!device)
>  		return NULL;
> 
> +	group = device->group;
>  	ctx = accfg_device_get_ctx(device);
>  	dfd = open(engine_base, O_PATH);
>  	if (dfd < 0)
> @@ -1109,12 +1110,13 @@ ACCFG_EXPORT int
> accfg_device_get_clients(struct accfg_device *device)  ACCFG_EXPORT int
> accfg_device_set_token_limit(struct accfg_device *dev, int val)  {
>  	struct accfg_ctx *ctx;
> -	char *path = dev->device_buf;
> +	char *path;
>  	char buf[SYSFS_ATTR_SIZE];
> 
>  	if (!dev)
>  		return -EINVAL;
> 
> +	path = dev->device_buf;
>  	ctx = accfg_device_get_ctx(dev);
> 
>  	if (sprintf(path, "%s/token_limit", dev->device_path) >= diff --git
> a/accfg/list.c b/accfg/list.c index c22da41..2234e65 100644
> --- a/accfg/list.c
> +++ b/accfg/list.c
> @@ -56,8 +56,7 @@ static struct json_object *group_to_json(struct
> accfg_group *group,
>  	struct json_object *jobj = NULL;
>  	struct accfg_device *dev = NULL;
> 
> -	if (group)
> -		dev = accfg_group_get_device(group);
> +	dev = accfg_group_get_device(group);
> 
>  	if (!jgroup)
>  		return NULL;
> --
> 2.26.2