From d9ddd3e7914e2239d00ae2c31c048b6eaf26ee60 Mon Sep 17 00:00:00 2001 From: Rob Meades Date: Fri, 30 Sep 2022 14:13:38 +0100 Subject: [PATCH] Cellular change only: re-entrant versions of file list functions. (#724) Re-entrant versions of the file list functions, uCellFileListFirst_r(), uCellFileListNext_r() and uCellFileListLast_r(), are added. These are required for the up-coming HTTP Client API. --- cell/api/u_cell_file.h | 53 ++++++++++++- cell/src/u_cell_file.c | 53 +++++++++---- cell/test/u_cell_file_test.c | 141 ++++++++++++++++++++++++++++++++++- 3 files changed, 230 insertions(+), 17 deletions(-) diff --git a/cell/api/u_cell_file.h b/cell/api/u_cell_file.h index 8ae872fe..57a4f372 100644 --- a/cell/api/u_cell_file.h +++ b/cell/api/u_cell_file.h @@ -178,7 +178,8 @@ int32_t uCellFileDelete(uDeviceHandle_t cellHandle, /** Get the description of file stored on the file system; uCellFileListNext() * should be called repeatedly to iterate through subsequent entries in * the list. This function is not thread-safe in that there is a single - * list of names for any given atHandle. + * list of names for any given cellHandle: for a re-entrant version see + * uCellFileListFirst_r() / uCellFileListNext_r() / uCellFileListLast_r(). * * For instance, to print out the names of all stored files on file system: * @@ -235,6 +236,56 @@ int32_t uCellFileListNext(uDeviceHandle_t cellHandle, */ void uCellFileListLast(uDeviceHandle_t cellHandle); +/** As uCellFileListFirst() but re-entrant; you must provide + * storage for the pointer ppRentrant, something like: + * + * ``` + * char fileName[U_CELL_FILE_NAME_MAX_LENGTH + 1]; + * void *pReentrant; + * + * for (int32_t x = uCellFileListFirst_r(handle, fileName, &pReentrant); + * x >= 0; + * x = uCellFileListNext_r(fileName, &pReentrant)) { + * printf("%s\n", fileName); + * } + * ``` + * + * @param cellHandle the handle of the cellular instance. + * @param[out] pFileName pointer to somewhere to store the result; + * at least #U_CELL_FILE_NAME_MAX_LENGTH + 1 bytes + * of storage must be provided. + * @param[out] ppRentrant pointer to a place where this function + can store its rentrancy pointer. + * @return the total number of file names in the list + * or negative error code. + */ +int32_t uCellFileListFirst_r(uDeviceHandle_t cellHandle, + char *pFileName, void **ppRentrant); + +/** As uCellFileListNext() but re-entrant; you must pass the + * re-entrancy pointer that was passed to uCellFileListFirst_r() + * to this function. + * + * @param[out] pFileName pointer to somewhere to store the result; + * at least #U_CELL_FILE_NAME_MAX_LENGTH + 1 + * bytes of storage must be provided. + * @param[in,out] ppRentrant pointer to the rentrancy pointer that + * was passed to uCellFileListFirst_r(). + * @return the number of entries remaining *after* + * this one has been read or negative error + * code. + */ +int32_t uCellFileListNext_r(char *pFileName, void **ppRentrant); + +/** As uCellFileListLast() but re-entrant; you must pass the + * re-entrancy pointer that was passed to uCellFileListFirst_r() + * to this function. + * + * @param[in] ppRentrant pointer to the rentrancy pointer that was + * passed to uCellFileListFirst_r(). + */ +void uCellFileListLast_r(void **ppRentrant); + #ifdef __cplusplus } #endif diff --git a/cell/src/u_cell_file.c b/cell/src/u_cell_file.c index 758fdabe..628f9115 100644 --- a/cell/src/u_cell_file.c +++ b/cell/src/u_cell_file.c @@ -413,6 +413,28 @@ int32_t uCellFileDelete(uDeviceHandle_t cellHandle, // Get the name of the first file stored on file system. int32_t uCellFileListFirst(uDeviceHandle_t cellHandle, char *pFileName) +{ + return uCellFileListFirst_r(cellHandle, pFileName, (void **) &gpFileList); +} + +// Return subsequent file name in the list. +int32_t uCellFileListNext(uDeviceHandle_t cellHandle, + char *pFileName) +{ + (void) cellHandle; + return uCellFileListNext_r(pFileName, (void **) &gpFileList); +} + +// Free memory from list. +void uCellFileListLast(uDeviceHandle_t cellHandle) +{ + (void) cellHandle; + uCellFileListLast_r((void **) &gpFileList); +} + +// Get the name of the first file stored on file system, re-entrant version. +int32_t uCellFileListFirst_r(uDeviceHandle_t cellHandle, + char *pFileName, void **ppRentrant) { int32_t errorCode = (int32_t) U_ERROR_COMMON_NOT_INITIALISED; uCellPrivateInstance_t *pInstance; @@ -421,10 +443,12 @@ int32_t uCellFileListFirst(uDeviceHandle_t cellHandle, U_PORT_MUTEX_LOCK(gUCellPrivateMutex); + errorCode = (int32_t) U_ERROR_COMMON_INVALID_PARAMETER; pInstance = pUCellPrivateGetInstance(cellHandle); - if (pInstance != NULL) { + if ((pInstance != NULL) && (ppRentrant != NULL)) { + *ppRentrant = NULL; errorCode = uCellPrivateFileListFirst(pInstance, - &gpFileList, + (uCellPrivateFileListContainer_t **) ppRentrant, pFileName); } @@ -434,22 +458,22 @@ int32_t uCellFileListFirst(uDeviceHandle_t cellHandle, return errorCode; } -// Return subsequent file name in the list. -int32_t uCellFileListNext(uDeviceHandle_t cellHandle, - char *pFileName) +// Return subsequent file name in the list, re-entrant version. +int32_t uCellFileListNext_r(char *pFileName, void **ppRentrant) { int32_t errorCode = (int32_t) U_ERROR_COMMON_NOT_INITIALISED; - // This parameter is no longer needed - (void) cellHandle; - if (gUCellPrivateMutex != NULL) { // Though this doesn't use the instance pointer we can // use the cellular API mutex to protect the linked list U_PORT_MUTEX_LOCK(gUCellPrivateMutex); - errorCode = uCellPrivateFileListNext(&gpFileList, pFileName); + errorCode = (int32_t) U_ERROR_COMMON_INVALID_PARAMETER; + if (ppRentrant != NULL) { + errorCode = uCellPrivateFileListNext((uCellPrivateFileListContainer_t **) ppRentrant, + pFileName); + } U_PORT_MUTEX_UNLOCK(gUCellPrivateMutex); } @@ -457,19 +481,18 @@ int32_t uCellFileListNext(uDeviceHandle_t cellHandle, return errorCode; } -// Free memory from list. -void uCellFileListLast(uDeviceHandle_t cellHandle) +// Free memory from list, re-entrant version. +void uCellFileListLast_r(void **ppRentrant) { - // This parameter is no longer needed - (void) cellHandle; - if (gUCellPrivateMutex != NULL) { // Though this doesn't use the instance pointer we can // use the cellular API mutex to protect the linked list U_PORT_MUTEX_LOCK(gUCellPrivateMutex); - uCellPrivateFileListLast(&gpFileList); + if (ppRentrant != NULL) { + uCellPrivateFileListLast((uCellPrivateFileListContainer_t **) ppRentrant); + } U_PORT_MUTEX_UNLOCK(gUCellPrivateMutex); } diff --git a/cell/test/u_cell_file_test.c b/cell/test/u_cell_file_test.c index 8d991bf5..9cf14c32 100644 --- a/cell/test/u_cell_file_test.c +++ b/cell/test/u_cell_file_test.c @@ -36,6 +36,7 @@ #include "stddef.h" // NULL, size_t etc. #include "stdint.h" // int32_t etc. #include "stdbool.h" +#include "stdio.h" // snprintf() #include "string.h" // memset(), strcmp() #include "u_cfg_sw.h" @@ -45,6 +46,9 @@ #include "u_error_common.h" +#include "u_port_clib_platform_specific.h" /* Integer stdio, must be included + before the other port files if + any print or scan function is used. */ #include "u_port.h" #include "u_port_debug.h" #include "u_port_os.h" // Required by u_cell_private.h @@ -74,7 +78,24 @@ /** The name of the file to use when testing. */ -#define U_CELL_FILE_TEST_FILE_NAME "test.txt" +#define U_CELL_FILE_TEST_FILE_NAME "test" + +/** strlen(U_CELL_FILE_TEST_FILE_NAME). + */ +#define U_CELL_FILE_TEST_FILE_NAME_LENGTH 4 + +/** The number of files to test for in the re-entrant listing + * version + */ +#define U_CELL_FILE_TEST_REENTRANT_NUM 3 + +/** The string to write to a file used in the re-entrant list testing. + */ +#define U_CELL_FILE_TEST_REENTRANT_STRING "delete me" + +/** strlen(U_CELL_FILE_TEST_REENTRANT_STRING). + */ +#define U_CELL_FILE_TEST_REENTRANT_STRING_SIZE 9 /* ---------------------------------------------------------------- * TYPES @@ -88,6 +109,37 @@ */ static uCellTestPrivate_t gHandles = U_CELL_TEST_PRIVATE_DEFAULTS; +/* ---------------------------------------------------------------- +* STATIC FUNCTIONS +* -------------------------------------------------------------- */ + +// Update a tracking array, used by cellFileListAllReentrant(). +static void updateTracker(const char *pFileName, bool *pTracker, + size_t size) +{ + size_t x; + + if (strlen(pFileName) == U_CELL_FILE_TEST_FILE_NAME_LENGTH + 1) { + x = strtol(pFileName + U_CELL_FILE_TEST_FILE_NAME_LENGTH, NULL, 10); + if (x < size) { + *(pTracker + x) = true; + } + } +} + +// Check a tracking array, used by cellFileListAllReentrant(); +// return true only if all elements are true. +static bool checkTracker(bool *pTracker, size_t size) +{ + bool isGood = true; + + for (size_t x = 0; (x < size) && isGood; x++, pTracker++) { + isGood = *pTracker; + } + + return isGood; +} + /* ---------------------------------------------------------------- * PUBLIC FUNCTIONS * -------------------------------------------------------------- */ @@ -446,6 +498,93 @@ U_PORT_TEST_FUNCTION("[cellFile]", "cellFileListAll") U_PORT_TEST_ASSERT(heapUsed <= 0); } +/** Test list all files, re-entrant version. + */ +U_PORT_TEST_FUNCTION("[cellFile]", "cellFileListAllReentrant") +{ + int32_t heapUsed; + uDeviceHandle_t cellHandle; + int32_t result = U_CELL_FILE_TEST_REENTRANT_STRING_SIZE; + // Enough room for the test file name plus a single number + // plus a null terminator + size_t y = 0; + char buffer[U_CELL_FILE_TEST_FILE_NAME_LENGTH + 2]; + char *pFileName; + void *pRentrantOuter; + bool trackerOuter[U_CELL_FILE_TEST_REENTRANT_NUM] = {0}; + void *pRentrantInner; + bool trackerInner[U_CELL_FILE_TEST_REENTRANT_NUM]; + + pFileName = (char *) malloc(U_CELL_FILE_NAME_MAX_LENGTH + 1); + U_PORT_TEST_ASSERT(pFileName != NULL); + + // In case a previous test failed + uCellTestPrivateCleanup(&gHandles); + + // Obtain the initial heap size + heapUsed = uPortGetHeapFree(); + + // Do the standard preamble + U_PORT_TEST_ASSERT(uCellTestPrivatePreamble(U_CFG_TEST_CELL_MODULE_TYPE, + &gHandles, true) == 0); + cellHandle = gHandles.cellHandle; + + // Write the files we need to list + for (size_t x = 0; (x < U_CELL_FILE_TEST_REENTRANT_NUM) && + (result == U_CELL_FILE_TEST_REENTRANT_STRING_SIZE); x++) { + snprintf(buffer, sizeof(buffer), "%s%1d", U_CELL_FILE_TEST_FILE_NAME, x); + U_TEST_PRINT_LINE("writing file %s...", buffer); + result = uCellFileWrite(cellHandle, buffer, + U_CELL_FILE_TEST_REENTRANT_STRING, + U_CELL_FILE_TEST_REENTRANT_STRING_SIZE); + } + U_PORT_TEST_ASSERT(result == U_CELL_FILE_TEST_REENTRANT_STRING_SIZE); + + // List the files in two loops, one within the other, making sure + // that all files are listed in both loops on each run + U_TEST_PRINT_LINE("listing the files..."); + for (int32_t x = uCellFileListFirst_r(cellHandle, pFileName, &pRentrantOuter); + x >= 0; + x = uCellFileListNext_r(pFileName, &pRentrantOuter), y++) { + U_TEST_PRINT_LINE("outer loop: \"%s\".", pFileName); + updateTracker(pFileName, trackerOuter, sizeof(trackerOuter) / sizeof(trackerOuter[0])); + + memset(trackerInner, 0, sizeof(trackerInner)); + for (int32_t y = uCellFileListFirst_r(cellHandle, pFileName, &pRentrantInner); + y >= 0; + y = uCellFileListNext_r(pFileName, &pRentrantInner)) { + U_TEST_PRINT_LINE("inner loop: \"%s\".", pFileName); + updateTracker(pFileName, trackerInner, sizeof(trackerInner) / sizeof(trackerInner[0])); + } + U_PORT_TEST_ASSERT(checkTracker(trackerInner, sizeof(trackerInner) / sizeof(trackerInner[0]))); + uCellFileListLast_r(&pRentrantInner); + U_TEST_PRINT_LINE("inner loop, all files listed on run %d.", y + 1); + } + U_PORT_TEST_ASSERT(checkTracker(trackerOuter, sizeof(trackerOuter) / sizeof(trackerOuter[0]))); + uCellFileListLast_r(&pRentrantOuter); + U_TEST_PRINT_LINE("outer loop, all files listed."); + + // Delete the files again, for tidiness + for (size_t x = 0; x < U_CELL_FILE_TEST_REENTRANT_NUM; x++) { + snprintf(buffer, sizeof(buffer), "%s%1d", U_CELL_FILE_TEST_FILE_NAME, x); + U_TEST_PRINT_LINE("deleting file %s...", buffer); + U_PORT_TEST_ASSERT(uCellFileDelete(cellHandle, buffer) == 0); + } + + free(pFileName); + + // Do the standard postamble, leaving the module on for the next + // test to speed things up + uCellTestPrivatePostamble(&gHandles, false); + + // Check for memory leaks + heapUsed -= uPortGetHeapFree(); + U_TEST_PRINT_LINE("we have leaked %d byte(s).", heapUsed); + // heapUsed < 0 for the Zephyr case where the heap can look + // like it increases (negative leak) + U_PORT_TEST_ASSERT(heapUsed <= 0); +} + /** Test deleting file. */ U_PORT_TEST_FUNCTION("[cellFile]", "cellFileDelete")