From 479c39329143561e68fbd57cbcbb1e0c96be068b Mon Sep 17 00:00:00 2001 From: Windrow14 Date: Thu, 4 Jul 2024 10:22:00 +0800 Subject: [PATCH] fs/fat/fs_fat32.[c|h]: fix fseek bug when file size is multiple of cluster size Unbinding `ff_currentcluster` and `f_pos`: 1. Added `ff_pos` in `struct fat_file_s`. 2. Added function `fat_zero_cluster` for doing zeroing for gap between EOF and new position beyond EOF. 3. Added function `fat_get_sectors` for getting the sector where `f_pos` is located, allocting new cluster when `f_pos` is beyond EOF. 4. Modify function `fat_read`, and `fat_write` with above functions. 5. Remove redundant logics in `fat_seek` since now new cluster is allocated when writing instead of seeking. Signed-off-by: Yinzhe Wu Reviewed-by: Yuezhang Mo Reviewed-by: Jacky Cao Tested-by: Yinzhe Wu --- fs/fat/fs_fat32.c | 550 +++++++++++++++++++++++++--------------------- fs/fat/fs_fat32.h | 1 + 2 files changed, 296 insertions(+), 255 deletions(-) diff --git a/fs/fat/fs_fat32.c b/fs/fat/fs_fat32.c index d4a913f8cde50..e3b77e4ddc09e 100644 --- a/fs/fat/fs_fat32.c +++ b/fs/fat/fs_fat32.c @@ -54,6 +54,10 @@ # define OFF_MAX INT32_MAX #endif +#define ROUND_DOWN(a, b) ((a) & ~((b) - 1)) +#define ROUND_UP(a, b) (((a) + (b) - 1) & ~((b) - 1)) +#define DIV_ROUND_UP(a, b) (ROUND_UP(a, b) / (b)) + /**************************************************************************** * Private Function Prototypes ****************************************************************************/ @@ -470,6 +474,271 @@ static int fat_close(FAR struct file *filep) return ret; } +/**************************************************************************** + * Name: fat_zero_cluster + + * Description: + * Zero the data in a cluster. Use to zero the data in the gap between EOF + * and the write offset. + * + * Input Parameters: + * fs - A reference to the fat volume object instance + * cluster - Cluster index to be zeroed + * start - The starting position in the cluster + * end - The ending position in the cluster + * + * Returned Value: + * Zero (OK) is returned on success; A negated errno value is returned on + * any failure. + * + ****************************************************************************/ + +static int fat_zero_cluster(FAR struct fat_mountpt_s *fs, int cluster, + int start, int end) +{ + FAR uint8_t *buf; + int zero_len = fs->fs_hwsectorsize - (start & (fs->fs_hwsectorsize - 1)); + off_t i; + off_t sector = fat_cluster2sector(fs, cluster); + off_t start_sec = sector + start / fs->fs_hwsectorsize; + off_t end_sec = sector + DIV_ROUND_UP(end, fs->fs_hwsectorsize); + int ret; + + buf = kmm_malloc(fs->fs_hwsectorsize); + if (!buf) + { + return -ENOMEM; + } + + if (zero_len) + { + ret = fat_hwread(fs, buf, start_sec, 1); + if (ret < 0) + { + goto out; + } + + memset(buf + fs->fs_hwsectorsize - zero_len, 0, zero_len); + + ret = fat_hwwrite(fs, buf, start_sec, 1); + if (ret < 0) + { + goto out; + } + + start_sec++; + } + + memset(buf, 0, fs->fs_hwsectorsize - zero_len); + + for (i = start_sec; i < end_sec; i++) + { + ret = fat_hwwrite(fs, buf, i, 1); + if (ret < 0) + { + goto out; + } + } + + ret = OK; + +out: + kmm_free(buf); + + return ret; +} + +/**************************************************************************** + * Name: fat_get_sectors + * + * Description: + * Get the sector index where ->f_pos is located. This function will + * allocate new clusters if get sectors for writing and ->f_pos is out of + * EOF and zero the data in the gap between EOF and the write offset. + * + * Input Parameters: + * fs - A reference to the file + * read - True if get sectors for reading + * + * Output: + * ->ff_currentsector - the sector index where ->f_pos is located + * ->ff_currentcluster - the cluster index where ->f_pos is located + * ->ff_sectorsincluster - sectors remaining in cluster + * ->ff_startcluster - the first cluster of the file when writing an + * empty file + * + * Returned Value: + * Zero (OK) is returned on success; A negated errno value is returned on + * any failure. + * + ****************************************************************************/ + +static int fat_get_sectors(FAR struct file *filep, bool read) +{ + FAR struct inode *inode = filep->f_inode; + FAR struct fat_mountpt_s *fs = inode->i_private; + FAR struct fat_file_s *ff = filep->f_priv; + int i; + int num_clu; + int new_num_clu; + int num_traversed; + int cluster; + int ret; + int zero_start; + int zero_end; + int clu_size = fs->fs_fatsecperclus * fs->fs_hwsectorsize; + + num_clu = DIV_ROUND_UP(ff->ff_size, clu_size); + new_num_clu = DIV_ROUND_UP(filep->f_pos + 1, clu_size); + + if (ff->ff_startcluster == 0) + { + /* empty file */ + + cluster = 0; + num_traversed = 0; + } + else if (ff->ff_currentcluster >= 2 && + ff->ff_currentcluster < fs->fs_nclusters + 2 && + ff->ff_pos <= filep->f_pos) + { + /* fatchain is traversed but not reach filep->f_pos */ + + cluster = ff->ff_currentcluster; + num_traversed = ff->ff_pos / clu_size + 1; + } + else + { + /* Traverse the FATchain from the first cluster of the file */ + + cluster = ff->ff_startcluster; + num_traversed = 1; + } + + /* Traverse the existing chain */ + + for (i = num_traversed; i < num_clu && i < new_num_clu; i++) + { + cluster = fat_getcluster(fs, cluster); + + /* The chain is broken */ + + if (cluster < 2 || cluster >= fs->fs_nclusters + 2) + { + return -EIO; + } + } + + if (read) + { + goto out; + } + + /* The 3 areas should be zeroed. + * + * cluster cluster+1..N cluster+2..N+1 + * +-------------------+------------------+---------------------+ + * | | (1) | (2) | (3) | | | + * +-------------------+------------------+---------------------+ + * ^ ^ + * ff_size f_pos + */ + + /* zero area (1) */ + + if (i == num_clu && filep->f_pos > ff->ff_size && ff->ff_size) + { + zero_start = ff->ff_size & (clu_size - 1); + + if (num_clu == new_num_clu) + { + zero_end = filep->f_pos & (clu_size - 1); + } + else + { + zero_end = clu_size; + } + + fat_ffcacheinvalidate(fs, ff); + + ret = fat_zero_cluster(fs, cluster, zero_start, zero_end); + if (ret) + { + return ret; + } + } + + /* Append new clusters for writing */ + + for (; i < new_num_clu - 1; i++) + { + cluster = fat_extendchain(fs, cluster); + + if (cluster < 2 || cluster >= fs->fs_nclusters + 2) + { + return -EIO; + } + + /* zero area (2) */ + + ret = fat_zero_cluster(fs, cluster, 0, clu_size); + if (ret < 0) + { + return ret; + } + + if (ff->ff_startcluster == 0) + { + ff->ff_startcluster = cluster; + } + } + + if (i == new_num_clu - 1) + { + cluster = fat_extendchain(fs, cluster); + + if (cluster < 2 || cluster >= fs->fs_nclusters + 2) + { + return -EIO; + } + + /* zero area (3) */ + + zero_end = filep->f_pos & (clu_size -1); + if (zero_end) + { + ret = fat_zero_cluster(fs, cluster, 0, zero_end); + if (ret < 0) + { + return ret; + } + } + + if (ff->ff_startcluster == 0) + { + ff->ff_startcluster = cluster; + } + } + + if (filep->f_pos > ff->ff_size) + { + ff->ff_size = filep->f_pos; + } + +out: + ff->ff_currentcluster = cluster; + + ret = fat_currentsector(fs, ff, filep->f_pos); + if (ret < 0) + { + return ret; + } + + ff->ff_pos = ROUND_DOWN(filep->f_pos, clu_size); + + return 0; +} + /**************************************************************************** * Name: fat_read ****************************************************************************/ @@ -483,7 +752,6 @@ static ssize_t fat_read(FAR struct file *filep, FAR char *buffer, unsigned int bytesread; unsigned int readsize; size_t bytesleft; - int32_t cluster; FAR uint8_t *userbuffer = (FAR uint8_t *)buffer; int sectorindex; int ret; @@ -544,32 +812,19 @@ static ssize_t fat_read(FAR struct file *filep, FAR char *buffer, ret = 0; goto errout_with_lock; } - - /* Get the number of bytes left in the file */ - - bytesleft = ff->ff_size - filep->f_pos; - - /* Truncate read count so that it does not exceed the number of bytes left - * in the file. - */ - - if (buflen > bytesleft) + else { - buflen = bytesleft; - } + /* Get the number of bytes left in the file */ - /* Get the first sector to read from. */ + bytesleft = ff->ff_size - filep->f_pos; - if (!ff->ff_currentsector) - { - /* The current sector can be determined from the current cluster and - * the file offset. + /* Truncate read count so that it does not exceed the number of bytes + * left in the file. */ - ret = fat_currentsector(fs, ff, filep->f_pos); - if (ret < 0) + if (buflen > bytesleft) { - goto errout_with_lock; + buflen = bytesleft; } } @@ -585,26 +840,10 @@ static ssize_t fat_read(FAR struct file *filep, FAR char *buffer, { bytesread = 0; - /* Check if the current read stream has incremented to the next - * cluster boundary - */ - - if (ff->ff_sectorsincluster < 1) + ret = fat_get_sectors(filep, true); + if (ret < 0) { - /* Find the next cluster in the FAT. */ - - cluster = fat_getcluster(fs, ff->ff_currentcluster); - if (cluster < 2 || cluster >= fs->fs_nclusters + 2) - { - ret = -EINVAL; /* Not the right error */ - goto errout_with_lock; - } - - /* Setup to read the first sector from the new cluster */ - - ff->ff_currentcluster = cluster; - ff->ff_currentsector = fat_cluster2sector(fs, cluster); - ff->ff_sectorsincluster = fs->fs_fatsecperclus; + goto errout_with_lock; } #ifdef CONFIG_FAT_DIRECT_RETRY /* Warning avoidance */ @@ -731,7 +970,6 @@ static ssize_t fat_write(FAR struct file *filep, FAR const char *buffer, FAR struct inode *inode; FAR struct fat_mountpt_s *fs; FAR struct fat_file_s *ff; - int32_t cluster; unsigned int byteswritten; unsigned int writesize; FAR uint8_t *userbuffer = (FAR uint8_t *)buffer; @@ -791,32 +1029,6 @@ static ssize_t fat_write(FAR struct file *filep, FAR const char *buffer, goto errout_with_lock; } - /* Get the first sector to write to. */ - - if (!ff->ff_currentsector) - { - /* Has the starting cluster been defined? */ - - if (ff->ff_startcluster == 0) - { - /* No.. we have to create a new cluster chain */ - - ff->ff_startcluster = fat_createchain(fs); - ff->ff_currentcluster = ff->ff_startcluster; - ff->ff_sectorsincluster = fs->fs_fatsecperclus; - } - - /* The current sector can then be determined from the current cluster - * and the file offset. - */ - - ret = fat_currentsector(fs, ff, filep->f_pos); - if (ret < 0) - { - goto errout_with_lock; - } - } - /* Loop until either (1) all data has been transferred, or (2) an * error occurs. We assume we start with the current sector in * cache (ff_currentsector) @@ -827,36 +1039,10 @@ static ssize_t fat_write(FAR struct file *filep, FAR const char *buffer, while (buflen > 0) { - /* Check if the current write stream has incremented to the next - * cluster boundary - */ - - if (ff->ff_sectorsincluster < 1) + ret = fat_get_sectors(filep, false); + if (ret < 0) { - /* Extend the current cluster by one (unless lseek was used to - * move the file position back from the end of the file) - */ - - cluster = fat_extendchain(fs, ff->ff_currentcluster); - - /* Verify the cluster number */ - - if (cluster < 0) - { - ret = cluster; - goto errout_with_lock; - } - else if (cluster < 2 || cluster >= fs->fs_nclusters + 2) - { - ret = -ENOSPC; - goto errout_with_lock; - } - - /* Setup to write the first sector from the new cluster */ - - ff->ff_currentcluster = cluster; - ff->ff_sectorsincluster = fs->fs_fatsecperclus; - ff->ff_currentsector = fat_cluster2sector(fs, cluster); + goto errout_with_lock; } #ifdef CONFIG_FAT_DIRECT_RETRY /* Warning avoidance */ @@ -1005,13 +1191,13 @@ static ssize_t fat_write(FAR struct file *filep, FAR const char *buffer, byteswritten += writesize; buflen -= writesize; sectorindex = filep->f_pos & SEC_NDXMASK(fs); - } - /* The transfer has completed without error. Update the file size */ + /* Update the file size */ - if (filep->f_pos > ff->ff_size) - { - ff->ff_size = filep->f_pos; + if (filep->f_pos > ff->ff_size) + { + ff->ff_size = filep->f_pos; + } } nxmutex_unlock(&fs->fs_lock); @@ -1031,9 +1217,7 @@ static off_t fat_seek(FAR struct file *filep, off_t offset, int whence) FAR struct inode *inode; FAR struct fat_mountpt_s *fs; FAR struct fat_file_s *ff; - int32_t cluster; off_t position; - unsigned int clustersize; int ret; /* Sanity checks */ @@ -1080,6 +1264,13 @@ static off_t fat_seek(FAR struct file *filep, off_t offset, int whence) return -EINVAL; } + /* Invalid arguments are entered, returns an error. */ + + if (position < 0) + { + return -EINVAL; + } + /* Special case: We are seeking to the current position. This would * happen normally with ftell() which does lseek(fd, 0, SEEK_CUR) but can * also happen in other situation such as when SEEK_SET is used to assure @@ -1117,158 +1308,7 @@ static off_t fat_seek(FAR struct file *filep, off_t offset, int whence) goto errout_with_lock; } - /* Attempts to set the position beyond the end of file will - * work if the file is open for write access. - */ - - if (position > ff->ff_size && (ff->ff_oflags & O_WROK) == 0) - { - /* Otherwise, the position is limited to the file size */ - - position = ff->ff_size; - } - - /* Set file position to the beginning of the file (first cluster, - * first sector in cluster) - */ - - filep->f_pos = 0; - ff->ff_sectorsincluster = fs->fs_fatsecperclus; - - /* Get the start cluster of the file */ - - cluster = ff->ff_startcluster; - - /* Create a new cluster chain if the file does not have one (and - * if we are seeking beyond zero - */ - - if (!cluster && position > 0) - { - cluster = fat_createchain(fs); - if (cluster < 0) - { - ret = cluster; - goto errout_with_lock; - } - - ff->ff_startcluster = cluster; - } - - /* Move file position if necessary */ - - if (cluster) - { - /* If the file has a cluster chain, follow it to the - * requested position. - */ - - clustersize = fs->fs_fatsecperclus * fs->fs_hwsectorsize; - for (; ; ) - { - /* Skip over clusters prior to the one containing - * the requested position. - */ - - ff->ff_currentcluster = cluster; - if (position < clustersize) - { - break; - } - - /* Extend the cluster chain if write in enabled. NOTE: - * this is not consistent with the lseek description: - * "The lseek() function allows the file offset to be - * set beyond the end of the file (but this does not - * change the size of the file). If data is later written - * at this point, subsequent reads of the data in the - * gap (a "hole") return null bytes ('\0') until data - * is actually written into the gap." - */ - - if ((ff->ff_oflags & O_WROK) != 0) - { - /* Extend the cluster chain (fat_extendchain - * will follow the existing chain or add new - * clusters as needed. - */ - - cluster = fat_extendchain(fs, cluster); - } - else - { - /* Otherwise we can only follow the existing chain */ - - cluster = fat_getcluster(fs, cluster); - } - - if (cluster < 0) - { - /* An error occurred getting the cluster */ - - ret = cluster; - goto errout_with_lock; - } - - /* Zero means that there is no further clusters available - * in the chain. - */ - - if (cluster == 0) - { - /* At the position to the current location and - * break out. - */ - - position = clustersize; - break; - } - - if (cluster >= fs->fs_nclusters + 2) - { - ret = -ENOSPC; - goto errout_with_lock; - } - - /* Otherwise, update the position and continue looking */ - - filep->f_pos += clustersize; - position -= clustersize; - } - - /* We get here after we have found the sector containing - * the requested position. - * - * Save the new file position - */ - - filep->f_pos += position; - - /* Then get the current sector from the cluster and the offset - * into the cluster from the position - */ - - fat_currentsector(fs, ff, filep->f_pos); - - /* Load the sector corresponding to the position */ - - if ((position & SEC_NDXMASK(fs)) != 0) - { - ret = fat_ffcacheread(fs, ff, ff->ff_currentsector); - if (ret < 0) - { - goto errout_with_lock; - } - } - } - - /* If we extended the size of the file, then mark the file as modified. */ - - if ((ff->ff_oflags & O_WROK) != 0 && filep->f_pos > ff->ff_size) - { - ff->ff_size = filep->f_pos; - ff->ff_bflags |= FFBUFF_MODIFIED; - } + filep->f_pos = position; nxmutex_unlock(&fs->fs_lock); return OK; diff --git a/fs/fat/fs_fat32.h b/fs/fat/fs_fat32.h index e1d7ce43a93c5..1f3ad5261f2af 100644 --- a/fs/fat/fs_fat32.h +++ b/fs/fat/fs_fat32.h @@ -907,6 +907,7 @@ struct fat_file_s off_t ff_startcluster; /* Start cluster of file on media */ off_t ff_currentsector; /* Current sector being operated on */ off_t ff_cachesector; /* Current sector in the file buffer */ + off_t ff_pos; /* Current position in the file */ uint8_t *ff_buffer; /* File buffer (for partial sector accesses) */ };