Hi,
On 27.11.2019 03:26, AKASHI Takahiro wrote:
Thank you for the heads-up.
On Tue, Nov 26, 2019 at 11:57:29AM -0500, Tom Rini wrote:
On Tue, Nov 26, 2019 at 09:15:07AM +0100, Marek Szyprowski wrote:
The code for handing file overwrite incorrectly assumed that the file on disk is always contiguous. This resulted in corrupting disk structure every time when write to existing fragmented file happened. Fix this by adding proper check for cluster discontinuity and adjust chunk size on each partial write.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com Reviewed-by: Oleksandr Suvorov oleksandr.suvorov@toradex.com Reviewed-by: Lukasz Majewski lukma@denx.de
fs/fat/fat_write.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c index 729cf39630..6cfa5b4565 100644 --- a/fs/fat/fat_write.c +++ b/fs/fat/fat_write.c @@ -794,6 +794,8 @@ set_contents(fsdata *mydata, dir_entry *dentptr, loff_t pos, __u8 *buffer,
newclust = get_fatent(mydata, endclust);
if ((newclust - 1) != endclust)"newclust != (endclust + 1)" would be more intuitive? But it's just my preference.
Indeed.
break; if (IS_LAST_CLUST(newclust, mydata->fatsize)) break; if (CHECK_CLUST(newclust, mydata->fatsize)) {@@ -811,7 +813,7 @@ set_contents(fsdata *mydata, dir_entry *dentptr, loff_t pos, __u8 *buffer, offset = 0; else offset = pos - cur_pos;
wsize = min(cur_pos + actsize, filesize) - pos;
wsize = min_t(unsigned long long, actsize, filesize - cur_pos);This hunk is not directly related to the issue, is it?
It is partially related. I remember that it was not calculated correctly for the fragmented files and then discovered that there was one more case in which the current formula failed.
if (get_set_cluster(mydata, curclust, offset, buffer, wsize, &actsize)) { printf("Error get-and-setting cluster\n");@@ -824,8 +826,6 @@ set_contents(fsdata *mydata, dir_entry *dentptr, loff_t pos, __u8 *buffer, if (filesize <= cur_pos) break;
/* CHECK: newclust = get_fatent(mydata, endclust); */- if (IS_LAST_CLUST(newclust, mydata->fatsize)) /* no more clusters */ break;
Adding in Heinrich and Akashi-san for more review on this, thanks!
Otherwise, it looks good. Reviewed-by: AKASHI Takahiro takahiro.akashi@linaro.org
Best regards