On Wed, 3 May 2023 01:56:41 -0700
Srikanth Yalavarthi <syalavar...@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 <syalavar...@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().
