Re: [PATCH v3] app/mldev: add internal function for file read

[Stephen Hemminger <stephen@networkplumber.org>]

On Wed, 3 May 2023 01:56:41 -0700
Srikanth Yalavarthi <syalavarthi@marvell.com> wrote:

> Added internal function to read model, input and reference
> files with required error checks. This change fixes the
> unchecked return value and improper use of negative value
> issues reported by coverity scan for file read operations.
> 
> Coverity issue: 383742, 383743
> Fixes: f6661e6d9a3a ("app/mldev: validate model operations")
> Fixes: da6793390596 ("app/mldev: support inference validation")
> 
> Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
> ---
> v3:
> * Fix incorrect use of rte_free with free
> 
> v2:
> * Replace rte_malloc in ml_read_file with malloc
> 
> v1:
> * Initial patch
> 
>  app/test-mldev/test_common.c           | 59 ++++++++++++++++++++++++++
>  app/test-mldev/test_common.h           |  2 +
>  app/test-mldev/test_inference_common.c | 54 +++++++++--------------
>  app/test-mldev/test_model_common.c     | 39 ++++-------------
>  4 files changed, 90 insertions(+), 64 deletions(-)
> 
> diff --git a/app/test-mldev/test_common.c b/app/test-mldev/test_common.c
> index 016b31c6ba..d8a8e8a448 100644
> --- a/app/test-mldev/test_common.c
> +++ b/app/test-mldev/test_common.c
> @@ -5,12 +5,71 @@
>  #include <errno.h>
> 
>  #include <rte_common.h>
> +#include <rte_malloc.h>
>  #include <rte_memory.h>
>  #include <rte_mldev.h>
> 
>  #include "ml_common.h"
>  #include "test_common.h"
> 
> +int
> +ml_read_file(char *file, size_t *size, char **buffer)
> +{
> +	char *file_buffer = NULL;
> +	long file_size = 0;
> +	int ret = 0;
> +	FILE *fp;
> +
> +	fp = fopen(file, "r");
> +	if (fp == NULL) {
> +		ml_err("Failed to open file: %s\n", file);
> +		return -EIO;
> +	}
> +
> +	if (fseek(fp, 0, SEEK_END) == 0) {
> +		file_size = ftell(fp);
> +		if (file_size == -1) {
> +			ret = -EIO;
> +			goto error;
> +		}
> +
> +		file_buffer = malloc(file_size);
> +		if (file_buffer == NULL) {
> +			ml_err("Failed to allocate memory: %s\n", file);
> +			ret = -ENOMEM;
> +			goto error;
> +		}
> +
> +		if (fseek(fp, 0, SEEK_SET) != 0) {
> +			ret = -EIO;
> +			goto error;
> +		}
> +
> +		if (fread(file_buffer, sizeof(char), file_size, fp) != (unsigned long)file_size) {
> +			ml_err("Failed to read file : %s\n", file);
> +			ret = -EIO;
> +			goto error;
> +		}
> +		fclose(fp);
> +	} else {


Granted this is a test program. But why did you ignore my feedback that this
is the slowest way to read a file. Stdio requires extra buffering, use regular read() or
better yet mmap().