[v2,2/9] core/pldm/test : pldm file I/O init Self test

The patch contains self test for PLDM file I/O function "pldm_file_io_init()".
This patch test codeflow for PLDM command PLDM_GET_FILE_TABLE.

Signed-off-by: Abhishek Singh Tomar <abhishek@linux.ibm.com>
---
 core/pldm/pldm-file-io-requests.c |  12 ++-
 core/pldm/pldm.h                  |   2 +-
 core/pldm/test/Makefile.check     |  42 ++++++++
 core/pldm/test/test_pldm-fileio.c | 174 ++++++++++++++++++++++++++++++
 4 files changed, 228 insertions(+), 2 deletions(-)
 create mode 100644 core/pldm/test/Makefile.check
 create mode 100644 core/pldm/test/test_pldm-fileio.c

Le 07/05/2022 à 08:35, Abhishek Singh Tomar a écrit :
> The patch contains self test for PLDM file I/O function "pldm_file_io_init()".
> This patch test codeflow for PLDM command PLDM_GET_FILE_TABLE.
>
> Signed-off-by: Abhishek Singh Tomar<abhishek@linux.ibm.com>
> ---
>   core/pldm/pldm-file-io-requests.c |  12 ++-
>   core/pldm/pldm.h                  |   2 +-
>   core/pldm/test/Makefile.check     |  42 ++++++++
>   core/pldm/test/test_pldm-fileio.c | 174 ++++++++++++++++++++++++++++++
>   4 files changed, 228 insertions(+), 2 deletions(-)
>   create mode 100644 core/pldm/test/Makefile.check
>   create mode 100644 core/pldm/test/test_pldm-fileio.c
>
> diff --git a/core/pldm/pldm-file-io-requests.c b/core/pldm/pldm-file-io-requests.c
> index 94828fcd..50ddb0e4 100644
> --- a/core/pldm/pldm-file-io-requests.c
> +++ b/core/pldm/pldm-file-io-requests.c
> @@ -155,9 +155,15 @@ static int read_file_req(uint32_t file_handle, uint32_t file_length,
>   		file_req.length = MAX_TRANSFER_SIZE_BYTES;
>   	}
>
> +#ifndef __TEST__
>   	prlog(PR_TRACE, "%s - file_handle: %d, offset: 0x%x, size: 0x%llx num_transfers: %d\n",
>   			__func__, file_handle, file_req.offset,
>   			size, num_transfers);
> +#else
> +	prlog(PR_TRACE, "%s - file_handle: %d, offset: 0x%x, size: 0x%lx num_transfers: %d\n",
> +			__func__, file_handle, file_req.offset,
> +			size, num_transfers);
> +#endif

hum. I don't think it's wise to introduce this compilation flag

>   	for (i = 0; i < num_transfers; i++) {
>   		file_req.offset = offset + (i * MAX_TRANSFER_SIZE_BYTES);
> @@ -210,9 +216,13 @@ static int read_file_req(uint32_t file_handle, uint32_t file_length,
>   		total_read += resp_length;
>   		curr_buf += resp_length;
>   		free(response_msg);
> -
> +#ifndef __TEST__
>   		prlog(PR_TRACE, "%s - file_handle: %d, resp_length: 0x%x, total_read: 0x%llx\n",
>   			__func__, file_handle, resp_length, total_read);
> +#else
> +		prlog(PR_TRACE, "%s - file_handle: %d, resp_length: 0x%x, total_read: 0x%lx\n",
> +			__func__, file_handle, resp_length, total_read);
> +#endif

same comment here.

>   		if (total_read == size)
>   			break;
> diff --git a/core/pldm/pldm.h b/core/pldm/pldm.h
> index 81df9f8b..5ac9c443 100644
> --- a/core/pldm/pldm.h
> +++ b/core/pldm/pldm.h
> @@ -45,7 +45,7 @@ extern int watchdog_period_sec;
>    * @example enum_bit(1) = 0x00000002
>    * @example enum_bit(4) = 0x00000010
>    */
> -inline uint32_t enum_bit(unsigned int enumeration)
> +extern inline uint32_t enum_bit(unsigned int enumeration)
>   {
>   	return 1 << enumeration;
>   }
> diff --git a/core/pldm/test/Makefile.check b/core/pldm/test/Makefile.check
> new file mode 100644
> index 00000000..42d60993
> --- /dev/null
> +++ b/core/pldm/test/Makefile.check
> @@ -0,0 +1,42 @@
> +# -*-Makefile-*-
> +PLDM_TEST := core/pldm/test/test_pldm-fileio \
> +
> +LCOV_EXCLUDE += $(PLDM_TEST:%=%.c)
> +
> +.PHONY : core-pldm-check core-pldm-coverage
> +core-pldm-check: $(PLDM_TEST:%=%-check)
> +core-pldm-coverage: $(PLDM_TEST:%=%-gcov-run)
> +HOSTCFLAG_PLDM:= $(filter-out -Wdeclaration-after-statement,$(HOSTCFLAGS))
> +HOSTCFLAG_PLDM:= $(filter-out -Wstrict-prototypes,$(HOSTCFLAG_PLDM))
> +HOSTCFLAG_PLDM:= $(filter-out -Wjump-misses-init,$(HOSTCFLAG_PLDM))
> +HOSTCFLAG_PLDM:= $(filter-out -Wmissing-prototypes,$(HOSTCFLAG_PLDM))
> +HOSTCFLAG_PLDM:= $(filter-out -Wmissing-declarations,$(HOSTCFLAG_PLDM))
> +
> +check: core-pldm-check
> +coverage: core-pldm-coverage
> +
> +$(PLDM_TEST:%=%-gcov-run) : %-run: %
> +	$(call Q, TEST-COVERAGE ,$< , $<)
> +
> +$(PLDM_TEST:%=%-check) : %-check: %
> +	$(call Q, RUN-TEST ,$(VALGRIND) $<, $<)
> +
> +core/test/stubs.o: core/test/stubs.c
> +	$(call Q, HOSTCC ,$(HOSTCC) $(HOSTCFLAGS) -I . -I include -Wno-error=attributes -g -c -o core/test/stubs.o core/test/stubs.c, $<)
> +
> +$(PLDM_TEST) : % : %.c core/test/stubs.o
> +	$(call Q, HOSTCC ,$(HOSTCC) $(HOSTCFLAG_PLDM) -O0 -g -I include -I . -I pldm/libpldm/  -I libfdt -o $@ $< core/test/stubs.o -g, $<)
> +
> +$(PLDM_TEST:%=%-gcov): %-gcov : %.c %
> +	$(call Q, HOSTCC ,$(HOSTCC) $(HOSTFLAG_PLDM) $(HOSTGCOVCFLAGS) -I include -I . -I pldm/libpldm/ -I libfdt -lgcov -o $@ $<, $<)
> +
> +$(PLDM_TEST:%=%-gcov): % : $(%.d:-gcov=)
> +
> +-include $(wildcard core/pldm/test/*.d)
> +
> +clean: pldm-test-clean
> +
> +pldm-test-clean:
> +	$(RM) -f core/pldm/test/*.[od] $(PLDM_TEST) $(PLDM_TEST:%=%-gcov)
> +	$(RM) -f *.gcda *.gcno skiboot.info
> +	$(RM) -rf coverage-report
> diff --git a/core/pldm/test/test_pldm-fileio.c b/core/pldm/test/test_pldm-fileio.c
> new file mode 100644
> index 00000000..722a8bf6
> --- /dev/null
> +++ b/core/pldm/test/test_pldm-fileio.c

The file name must only content this character "-"

> @@ -0,0 +1,174 @@
> +// SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later
> +/*
> + * Copyright 2013-2019 IBM Corp.
> + */
> +

2022

> +
> +#include "common/test_pldm-common.c"
> +
> +#define TEST_FILE_IO_NAME "81e0066b.lid"
> +#define TEST_FILE_IO_HANDLE 11
> +#define TEST_FILE_IO_LENGTH 50
> +#define TEST_FILE_IO_BUF1 "This is Test buffer Open power Foundation"
> +
> +void *pldm_file_io_buff[TEST_FILE_IO_LENGTH];
> +uint32_t test_Filetable_entry_generate(uint8_t **file_attr_table);
> +int pldm_test_reply_request_file_io(void *request_msg, size_t request_len,
> +			void **response_msg, size_t *response_len);
> +
> +int pldm_test_reply_request(void *request_msg, size_t request_len,
> +		void **response_msg, size_t *response_len);
> +
> +
> +/*
> + * This function tries to duplicate BMC functionality for Pldm self test
> + * It will handle pldm response message
> + * For now we don't have any response
> + */
> +int pldm_test_verify_response(void *response_msg, size_t response_len)
> +{
> +	if (response_len > 0 || response_msg != NULL)
> +		return OPAL_PARAMETER;
> +

this previous test is not necessary.

> +	return OPAL_PARAMETER;
> +
> +}
> +
> +/*
> + * This function tries to duplicate BMC functionality for Pldm self test

"This function duplicates ..."

> + * This Genrate Filetable entry for self test
> + * The file table contains the list of files available and
> + * their attributes.
> + *
> + * Ex:
> + * {
> + *   "FileHandle": "11",
> + *   "FileNameLength": 12,
> + *   "FileName": "81e0066b.lid",
> + *   "FileSize": 589824,
> + *   "FileTraits": 6
> + * }
> + */
> +uint32_t test_Filetable_entry_generate(uint8_t **file_attr_table)
> +{

function names are lowercase

> +	struct pldm_file_attr_table_entry *pldm_file_attr_table_entry;
> +	uint8_t FileName[] = TEST_FILE_IO_NAME;
> +	uint32_t file_length = TEST_FILE_IO_LENGTH;
> +	int size;
> +
> +	/* calculate sizeof whole struct */
> +	size = sizeof(struct pldm_file_attr_table_entry *) + strlen(FileName)
> +			+ sizeof(file_length) - 1;
> +	*file_attr_table = malloc(size);

*file_attr_tablecan be null

> +
> +	pldm_file_attr_table_entry = (struct pldm_file_attr_table_entry *)*file_attr_table;
> +	pldm_file_attr_table_entry->file_handle = TEST_FILE_IO_HANDLE;
> +	pldm_file_attr_table_entry->file_name_length = strlen(FileName);
> +	memcpy(pldm_file_attr_table_entry->file_attr_table_nst, FileName,
> +		strlen(FileName));
> +
> +	memcpy(pldm_file_attr_table_entry->file_attr_table_nst + strlen(FileName),
> +			(uint8_t *)&file_length,	sizeof(file_length));
> +
> +	return size;
> +}
> +
> +/*
> + * This function tries to duplicate BMC functionality for Pldm self test
> + * It will only handle PLDM_OEM type request
> + * As fileio test will have only pldm request of type = PLDM_OEM
> + */
> +int pldm_test_reply_request(void *request_msg, size_t request_len,
> +		void **response_msg, size_t *response_len)
> +{
> +
> +	switch (((struct pldm_msg *)request_msg)->hdr.type) {
> +	case PLDM_OEM:
> +		return pldm_test_reply_request_file_io(request_msg, request_len,
> +			response_msg, response_len);
> +	default:
> +		printf("PLDM_TEST: Not equal to PLDM_OEM\n");
> +		return OPAL_PARAMETER;
> +	}
> +
> +
> +}
> +
> +
> +/*
> + * This function tries to duplicate BMC functionality for Pldm self test
> + * it tries to handle PLDM_REQUEST for fileio and reply with appropriate PLDM_RESPONSE
> + * message
> + */
> +int pldm_test_reply_request_file_io(void *request_msg, size_t request_len,
> +			void **response_msg, size_t *response_len)
> +{
> +	int ret;
> +	int  payload_len = 0;
> +	uint32_t transfer_handle;
> +	uint8_t transfer_opflag;
> +	uint8_t table_type;
> +	uint8_t *file_attr_table;
> +	uint32_t table_size;
> +
> +
> +	/* check command received and reply with appropriate pldm response message */
> +	switch (((struct pldm_msg *)request_msg)->hdr.command) {
> +	case PLDM_GET_FILE_TABLE:
> +
> +		payload_len = request_len - sizeof(struct pldm_msg_hdr);
> +
> +		ret = decode_get_file_table_req(request_msg, payload_len, &transfer_handle,
> +				&transfer_opflag, &table_type);
> +		if (ret != PLDM_SUCCESS)
> +			return ret;
> +
> +		/* Generate Filetable entry for self test */
> +		table_size = test_Filetable_entry_generate(&file_attr_table);
> +

May be, you could generate the filetable in the main function, in case 
of a new
PLDM command should be handled.

> +		*response_len = sizeof(struct pldm_msg_hdr)
> +			+ sizeof(struct pldm_get_file_table_resp)
> +			+ table_size - 1;
> +		*response_msg = malloc(*response_len);
> +

response_msg doesn't seem to be freed.

> +		ret = encode_get_file_table_resp(((struct pldm_msg *)request_msg)->hdr.instance_id,
> +				PLDM_SUCCESS, PLDM_GET_NEXTPART, PLDM_START_AND_END,
> +				file_attr_table, table_size, *response_msg);
> +		if (ret != PLDM_SUCCESS)
> +			return ret;
> +
> +		free(file_attr_table);
> +
> +		break;
> +
> +	default:
> +		return PLDM_ERROR_INVALID_DATA;
> +
> +	}
> +
> +	return PLDM_SUCCESS;
> +
> +
> +
> +}
> +
> +
> +int main(void)
> +{
> +	size_t ret;
> +
> +       /* Initialize test buffer for represent file with 0 */
> +	bzero(pldm_file_io_buff, TEST_FILE_IO_LENGTH);
> +

pldm_file_io_buff() is never used in this patch

> +
> +	/* Init PLDM File IO */
> +	ret = pldm_file_io_init();
> +	if (ret != PLDM_SUCCESS) {
> +		perror("pldm_file_io_write_file");

???

> +		return ret;
> +	}
> +
> +	return PLDM_SUCCESS;
> +}
> +
> +