From 507e273e1d6908d53d08b22477a4a601d060d8a3 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Tue, 31 May 2022 13:34:29 +0200 Subject: [PATCH] fatfs: fix incorrect mtime returned for files created during DST mktime function uses tm_isdst member as an indicator whether the time stamp is expected to be in daylight saving time (1) or not (0). FAT filesystem uses local time as mtime, so no information about DST is available from the filesystem. According to mktime documentation, tm_isdst can be set to -1, in which case the C library will try to determine if DST was or wasn't in effect at that time, and will set UTC time accordingly. Note that the conversion from UTC to local time and then back to UTC (time_t -> localtime_r -> FAT timestamp -> mktime -> time_t) does not always recover the same UTC time. In particular, the local time in the hour before DST comes into effect can be interpreted as "before DST" or "after DST", which would correspond to different UTC values. In this case which option the C library chooses is undefined. Closes https://github.com/espressif/esp-idf/issues/9039 Originally reported in https://github.com/espressif/arduino-esp32/issues/6786 --- components/fatfs/test/test_fatfs_common.c | 43 +++++++++++++++++---- components/fatfs/test/test_fatfs_common.h | 2 + components/fatfs/test/test_fatfs_spiflash.c | 7 ++++ components/fatfs/vfs/vfs_fat.c | 8 +++- 4 files changed, 52 insertions(+), 8 deletions(-) diff --git a/components/fatfs/test/test_fatfs_common.c b/components/fatfs/test/test_fatfs_common.c index 6b44bf03ce..aab5371747 100644 --- a/components/fatfs/test/test_fatfs_common.c +++ b/components/fatfs/test/test_fatfs_common.c @@ -382,13 +382,14 @@ void test_fatfs_ftruncate_file(const char* filename) void test_fatfs_stat(const char* filename, const char* root_dir) { - struct tm tm; - tm.tm_year = 2017 - 1900; - tm.tm_mon = 11; - tm.tm_mday = 8; - tm.tm_hour = 19; - tm.tm_min = 51; - tm.tm_sec = 10; + struct tm tm = { + .tm_year = 2017 - 1900, + .tm_mon = 11, + .tm_mday = 8, + .tm_hour = 19, + .tm_min = 51, + .tm_sec = 10 + }; time_t t = mktime(&tm); printf("Setting time: %s", asctime(&tm)); struct timeval now = { .tv_sec = t }; @@ -413,6 +414,34 @@ void test_fatfs_stat(const char* filename, const char* root_dir) TEST_ASSERT_FALSE(st.st_mode & S_IFREG); } +void test_fatfs_mtime_dst(const char* filename, const char* root_dir) +{ + struct timeval tv = { 1653638041, 0 }; + settimeofday(&tv, NULL); + setenv("TZ", "MST7MDT,M3.2.0,M11.1.0", 1); + tzset(); + + struct tm tm; + time_t sys_time = tv.tv_sec; + localtime_r(&sys_time, &tm); + printf("Setting time: %s", asctime(&tm)); + + test_fatfs_create_file_with_text(filename, "foo\n"); + + struct stat st; + TEST_ASSERT_EQUAL(0, stat(filename, &st)); + + time_t mtime = st.st_mtime; + struct tm mtm; + localtime_r(&mtime, &mtm); + printf("File time: %s", asctime(&mtm)); + + TEST_ASSERT(llabs(mtime - sys_time) < 2); // fatfs library stores time with 2 second precision + + unsetenv("TZ"); + tzset(); +} + void test_fatfs_utime(const char* filename, const char* root_dir) { struct stat achieved_stat; diff --git a/components/fatfs/test/test_fatfs_common.h b/components/fatfs/test/test_fatfs_common.h index d356f900a6..4789985712 100644 --- a/components/fatfs/test/test_fatfs_common.h +++ b/components/fatfs/test/test_fatfs_common.h @@ -55,6 +55,8 @@ void test_fatfs_ftruncate_file(const char* path); void test_fatfs_stat(const char* filename, const char* root_dir); +void test_fatfs_mtime_dst(const char* filename, const char* root_dir); + void test_fatfs_utime(const char* filename, const char* root_dir); void test_fatfs_unlink(const char* filename); diff --git a/components/fatfs/test/test_fatfs_spiflash.c b/components/fatfs/test/test_fatfs_spiflash.c index 6f1e052fdf..9fb5268b4a 100644 --- a/components/fatfs/test/test_fatfs_spiflash.c +++ b/components/fatfs/test/test_fatfs_spiflash.c @@ -125,6 +125,13 @@ TEST_CASE("(WL) stat returns correct values", "[fatfs][wear_levelling]") test_teardown(); } +TEST_CASE("(WL) stat returns correct mtime if DST is enabled", "[fatfs][wear_levelling]") +{ + test_setup(); + test_fatfs_mtime_dst("/spiflash/statdst.txt", "/spiflash"); + test_teardown(); +} + TEST_CASE("(WL) utime sets modification time", "[fatfs][wear_levelling]") { test_setup(); diff --git a/components/fatfs/vfs/vfs_fat.c b/components/fatfs/vfs/vfs_fat.c index ddcc711d3d..600af9dff0 100644 --- a/components/fatfs/vfs/vfs_fat.c +++ b/components/fatfs/vfs/vfs_fat.c @@ -613,7 +613,13 @@ static int vfs_fat_stat(void* ctx, const char * path, struct stat * st) .tm_year = fdate.year + 80, .tm_sec = ftime.sec * 2, .tm_min = ftime.min, - .tm_hour = ftime.hour + .tm_hour = ftime.hour, + /* FAT doesn't keep track if the time was DST or not, ask the C library + * to try to figure this out. Note that this may yield incorrect result + * in the hour before the DST comes in effect, when the local time can't + * be converted to UTC uniquely. + */ + .tm_isdst = -1 }; st->st_mtime = mktime(&tm); st->st_atime = 0;