diff --git a/ff_file.c b/ff_file.c index fbae1da..55bcda9 100644 --- a/ff_file.c +++ b/ff_file.c @@ -2477,6 +2477,11 @@ int32_t FF_Write( FF_FILE * pxFile, } else { + if( nBytesWritten > 0U ) + { + pxFile->ulValidFlags |= FF_VALID_FLAG_MODIFIED; + } + lResult = ( int32_t ) ( nBytesWritten / ulElementSize ); } @@ -2550,6 +2555,7 @@ int32_t FF_PutC( FF_FILE * pxFile, if( FF_isERR( xResult ) == pdFALSE ) { + pxFile->ulValidFlags |= FF_VALID_FLAG_MODIFIED; xResult = ( FF_Error_t ) ucValue; } } while( pdFALSE ); @@ -3143,16 +3149,37 @@ FF_Error_t FF_Close( FF_FILE * pxFile ) xError = FF_GetEntry( pxFile->pxIOManager, pxFile->usDirEntry, pxFile->ulDirCluster, &xOriginalEntry ); /* Now update the directory entry */ - if( ( FF_isERR( xError ) == pdFALSE ) && - ( ( pxFile->ulFileSize != xOriginalEntry.ulFileSize ) || ( pxFile->ulFileSize == 0UL ) ) ) + if( FF_isERR( xError ) == pdFALSE ) { - if( pxFile->ulFileSize == 0UL ) + BaseType_t xUpdateDirectoryEntry = pdFALSE; + + if( ( pxFile->ulFileSize != xOriginalEntry.ulFileSize ) || ( pxFile->ulFileSize == 0UL ) ) { - xOriginalEntry.ulObjectCluster = 0; + if( pxFile->ulFileSize == 0UL ) + { + xOriginalEntry.ulObjectCluster = 0; + } + + xOriginalEntry.ulFileSize = pxFile->ulFileSize; + xUpdateDirectoryEntry = pdTRUE; } - xOriginalEntry.ulFileSize = pxFile->ulFileSize; - xError = FF_PutEntry( pxFile->pxIOManager, pxFile->usDirEntry, pxFile->ulDirCluster, &xOriginalEntry, NULL ); + #if ( ffconfigTIME_SUPPORT != 0 ) && ( ffconfigUPDATE_FILE_MODIFIED_TIME_ON_CLOSE != 0 ) + { + if( ( pxFile->ulValidFlags & FF_VALID_FLAG_MODIFIED ) != 0U ) + { + /* Reuse this directory-entry write so timestamp + * maintenance does not add another disk update. */ + ( void ) FF_GetSystemTime( &xOriginalEntry.xModifiedTime ); + xUpdateDirectoryEntry = pdTRUE; + } + } + #endif /* ffconfigTIME_SUPPORT && ffconfigUPDATE_FILE_MODIFIED_TIME_ON_CLOSE */ + + if( xUpdateDirectoryEntry != pdFALSE ) + { + xError = FF_PutEntry( pxFile->pxIOManager, pxFile->usDirEntry, pxFile->ulDirCluster, &xOriginalEntry, NULL ); + } } } } @@ -3230,6 +3257,7 @@ FF_Error_t FF_Close( FF_FILE * pxFile ) FF_Error_t FF_SetEof( FF_FILE * pxFile ) { FF_Error_t xError; + uint32_t ulOriginalFileSize = pxFile->ulFileSize; /* Check if the file was not deleted and if it was opened with write permissions: */ if( ( ( pxFile->ulValidFlags & FF_VALID_FLAG_DELETED ) == 0 ) && @@ -3245,6 +3273,11 @@ FF_Error_t FF_SetEof( FF_FILE * pxFile ) { xError = FF_ERR_NONE; } + + if( ( FF_isERR( xError ) == pdFALSE ) && ( pxFile->ulFileSize != ulOriginalFileSize ) ) + { + pxFile->ulValidFlags |= FF_VALID_FLAG_MODIFIED; + } } else { diff --git a/include/FreeRTOSFATConfigDefaults.h b/include/FreeRTOSFATConfigDefaults.h index 8bc4836..d193ed3 100644 --- a/include/FreeRTOSFATConfigDefaults.h +++ b/include/FreeRTOSFATConfigDefaults.h @@ -218,6 +218,16 @@ #define ffconfigTIME_SUPPORT 0 #endif +#if !defined( ffconfigUPDATE_FILE_MODIFIED_TIME_ON_CLOSE ) + +/* Set to 1 to update a file's modified timestamp when closing a file that was + * changed through FF_Write(), FF_PutC(), or FF_SetEof(). + * + * Set to 0 to preserve the historical behaviour and avoid extra directory + * entry writes on close. */ + #define ffconfigUPDATE_FILE_MODIFIED_TIME_ON_CLOSE 0 +#endif + #if !defined( ffconfigREMOVABLE_MEDIA ) /* Set to 1 if the media is removable (such as a memory card). diff --git a/include/ff_file.h b/include/ff_file.h index d70e915..0d8d8fb 100644 --- a/include/ff_file.h +++ b/include/ff_file.h @@ -82,6 +82,7 @@ typedef struct _FF_FILE #define FF_VALID_FLAG_INVALID 0x00000001U #define FF_VALID_FLAG_DELETED 0x00000002U #define FF_VALID_FLAG_EXTENDED 0x00000004U +#define FF_VALID_FLAG_MODIFIED 0x00000008U /*---------- PROTOTYPES */ /* PUBLIC (Interfaces): */ diff --git a/test/unit-test/CMakeLists.txt b/test/unit-test/CMakeLists.txt index 46eb7e8..add095b 100644 --- a/test/unit-test/CMakeLists.txt +++ b/test/unit-test/CMakeLists.txt @@ -108,6 +108,69 @@ create_test( ${utest_name} "${utest_link_list}" "${utest_dep_list}" "${test_include_directories}" ) +set( coverage_dep_list ${utest_name} ) + +set( project_name "ff_file" ) + +# ===================== Mocks ================================================ +# Keep the ff_file unit focused on FF_Close(). The module is built with +# function sections so the linker can discard unrelated file APIs and their +# dependencies. +set( mock_list "" ) +list( APPEND mock_list + ${MODULE_ROOT_DIR}/include/ff_locking.h + ${MODULE_ROOT_DIR}/include/ff_dir.h + ${MODULE_ROOT_DIR}/include/ff_fat.h + ${MODULE_ROOT_DIR}/include/ff_ioman.h + ${MODULE_ROOT_DIR}/include/ff_time.h ) + +set( mock_include_list ${FAT_TEST_INCLUDE_DIRS} ) +set( mock_define_list "" ) + +# ================== Module under test ======================================= +set( real_source_files "" ) +list( APPEND real_source_files + ${MODULE_ROOT_DIR}/ff_file.c ) + +set( real_include_directories ${FAT_TEST_INCLUDE_DIRS} ) + +# ===================== Test code ============================================ +set( test_include_directories ${FAT_TEST_INCLUDE_DIRS} ) + +# ============================================================================== +set( mock_name "${project_name}_mock" ) +set( real_name "${project_name}_real" ) + +create_mock_list( ${mock_name} + "${mock_list}" + "${MODULE_ROOT_DIR}/tools/cmock/project.yml" + "${mock_include_list}" + "${mock_define_list}" ) + +create_real_library( ${real_name} + "${real_source_files}" + "${real_include_directories}" + "${mock_name}" ) +target_compile_options( ${real_name} PRIVATE -ffunction-sections -fdata-sections ) + +set( utest_dep_list "" ) +list( APPEND utest_dep_list ${real_name} ) + +set( utest_link_list "" ) +list( APPEND utest_link_list + lib${real_name}.a + -l${mock_name} + -Wl,--gc-sections ) + +set( utest_name "${project_name}_utest" ) +set( utest_source "${UNIT_TEST_DIR}/ff_file_utest.c" ) + +create_test( ${utest_name} + ${utest_source} + "${utest_link_list}" + "${utest_dep_list}" + "${test_include_directories}" ) +list( APPEND coverage_dep_list ${utest_name} ) # ------------------------------------------------------------------------------ # `coverage` target: run the tests and collect lcov data into coverage.info. @@ -115,6 +178,6 @@ create_test( ${utest_name} add_custom_target( coverage COMMAND ${CMAKE_COMMAND} -DCMAKE_BINARY_DIR=${CMAKE_BINARY_DIR} -P ${MODULE_ROOT_DIR}/tools/cmock/coverage.cmake - DEPENDS ${utest_name} + DEPENDS ${coverage_dep_list} WORKING_DIRECTORY ${CMAKE_BINARY_DIR} COMMENT "Running unit tests and collecting coverage" ) diff --git a/test/unit-test/config/FreeRTOSFATConfig.h b/test/unit-test/config/FreeRTOSFATConfig.h index 5000614..09ea43e 100644 --- a/test/unit-test/config/FreeRTOSFATConfig.h +++ b/test/unit-test/config/FreeRTOSFATConfig.h @@ -39,6 +39,9 @@ * with the generated mocks. */ #define ffconfig64_NUM_SUPPORT ( 1 ) +#define ffconfigTIME_SUPPORT ( 1 ) +#define ffconfigUPDATE_FILE_MODIFIED_TIME_ON_CLOSE ( 1 ) + /* All other ffconfig values fall back to FreeRTOSFATConfigDefaults.h. */ #endif /* FREERTOS_FAT_CONFIG_H */ diff --git a/test/unit-test/ff_file_utest.c b/test/unit-test/ff_file_utest.c new file mode 100644 index 0000000..ffe7488 --- /dev/null +++ b/test/unit-test/ff_file_utest.c @@ -0,0 +1,138 @@ +/* + * Unit tests for file-close metadata updates in ff_file.c. + * + * SPDX-License-Identifier: MIT + */ + +#include +#include + +#include "unity.h" + +#include "mock_ff_locking.h" +#include "mock_ff_dir.h" +#include "mock_ff_ioman.h" +#include "mock_ff_time.h" + +#include "ff_headers.h" + +#define TEST_SECTOR_SIZE ( 512U ) +#define TEST_SECTORS_PER_CLUSTER ( 4U ) +#define TEST_DIR_ENTRY ( 3U ) +#define TEST_DIR_CLUSTER ( 9U ) +#define TEST_FILE_SIZE ( 123U ) +#define TEST_UPDATED_YEAR ( 2026U ) + +static FF_IOManager_t xIOManager; + +static void prvInitFile( FF_FILE * pxFile ) +{ + memset( &xIOManager, 0, sizeof( xIOManager ) ); + memset( pxFile, 0, sizeof( *pxFile ) ); + + xIOManager.usSectorSize = TEST_SECTOR_SIZE; + xIOManager.xPartition.usBlkSize = TEST_SECTOR_SIZE; + xIOManager.xPartition.ulSectorsPerCluster = TEST_SECTORS_PER_CLUSTER; + xIOManager.FirstFile = pxFile; + + pxFile->pxIOManager = &xIOManager; + pxFile->ulFileSize = TEST_FILE_SIZE; + pxFile->ucMode = FF_MODE_WRITE; + pxFile->usDirEntry = TEST_DIR_ENTRY; + pxFile->ulDirCluster = TEST_DIR_CLUSTER; +} + +static void prvExpectValidHandleCheck( void ) +{ + FF_PendSemaphore_Expect( xIOManager.pvSemaphore ); + FF_ReleaseSemaphore_Expect( xIOManager.pvSemaphore ); +} + +static void prvExpectCloseListRemoval( void ) +{ + FF_PendSemaphore_Expect( xIOManager.pvSemaphore ); + FF_ReleaseSemaphore_Expect( xIOManager.pvSemaphore ); +} + +void setUp( void ) +{ +} + +void tearDown( void ) +{ +} + +void test_FF_Close_updates_modified_time_for_written_file_when_size_is_unchanged( void ) +{ + FF_FILE * pxFile = malloc( sizeof( *pxFile ) ); + FF_DirEnt_t xEntry; + FF_DirEnt_t xExpectedEntry; + FF_SystemTime_t xUpdatedTime = { 0 }; + + TEST_ASSERT_NOT_NULL( pxFile ); + prvInitFile( pxFile ); + pxFile->ulValidFlags = FF_VALID_FLAG_MODIFIED; + + memset( &xEntry, 0, sizeof( xEntry ) ); + xEntry.ulFileSize = TEST_FILE_SIZE; + xEntry.xModifiedTime.Year = 2001U; + + xExpectedEntry = xEntry; + xUpdatedTime.Year = TEST_UPDATED_YEAR; + xUpdatedTime.Month = 7U; + xUpdatedTime.Day = 1U; + xUpdatedTime.Hour = 12U; + xExpectedEntry.xModifiedTime = xUpdatedTime; + + prvExpectValidHandleCheck(); + FF_GetEntry_ExpectAndReturn( &xIOManager, + TEST_DIR_ENTRY, + TEST_DIR_CLUSTER, + NULL, + FF_ERR_NONE ); + FF_GetEntry_IgnoreArg_pxDirent(); + FF_GetEntry_ReturnThruPtr_pxDirent( &xEntry ); + FF_GetSystemTime_ExpectAndReturn( NULL, 0 ); + FF_GetSystemTime_IgnoreArg_pxTime(); + FF_GetSystemTime_ReturnThruPtr_pxTime( &xUpdatedTime ); + FF_PutEntry_ExpectWithArrayAndReturn( &xIOManager, + 1, + TEST_DIR_ENTRY, + TEST_DIR_CLUSTER, + &xExpectedEntry, + 1, + NULL, + 0, + FF_ERR_NONE ); + FF_PutEntry_IgnoreArg_pxIOManager(); + FF_PutEntry_IgnoreArg_pucContents(); + prvExpectCloseListRemoval(); + FF_FlushCache_ExpectAndReturn( &xIOManager, FF_ERR_NONE ); + + TEST_ASSERT_EQUAL( FF_ERR_NONE, FF_Close( pxFile ) ); +} + +void test_FF_Close_does_not_write_directory_entry_when_file_is_not_modified( void ) +{ + FF_FILE * pxFile = malloc( sizeof( *pxFile ) ); + FF_DirEnt_t xEntry; + + TEST_ASSERT_NOT_NULL( pxFile ); + prvInitFile( pxFile ); + + memset( &xEntry, 0, sizeof( xEntry ) ); + xEntry.ulFileSize = TEST_FILE_SIZE; + + prvExpectValidHandleCheck(); + FF_GetEntry_ExpectAndReturn( &xIOManager, + TEST_DIR_ENTRY, + TEST_DIR_CLUSTER, + NULL, + FF_ERR_NONE ); + FF_GetEntry_IgnoreArg_pxDirent(); + FF_GetEntry_ReturnThruPtr_pxDirent( &xEntry ); + prvExpectCloseListRemoval(); + FF_FlushCache_ExpectAndReturn( &xIOManager, FF_ERR_NONE ); + + TEST_ASSERT_EQUAL( FF_ERR_NONE, FF_Close( pxFile ) ); +}