Am 26.05.2024 um 11:56 hat Amjad Alsharafi geschrieben:
> Before this commit, the behavior when calling `commit_one_file` for
> example with `offset=0x2000` (second cluster), what will happen is that
> we won't fetch the next cluster from the fat, and instead use the first
> cluster for the read operation.
>
> This is due to off-by-one error here, where `i=0x2000 !< offset=0x2000`,
> thus not fetching the next cluster.
>
> Signed-off-by: Amjad Alsharafi <amjadsharaf...@gmail.com>
Reviewed-by: Kevin Wolf <kw...@redhat.com>
Tested-by: Kevin Wolf <kw...@redhat.com>
> diff --git a/block/vvfat.c b/block/vvfat.c
> index 9d050ba3ae..ab342f0743 100644
> --- a/block/vvfat.c
> +++ b/block/vvfat.c
> @@ -2525,7 +2525,7 @@ commit_one_file(BDRVVVFATState* s, int dir_index,
> uint32_t offset)
> return -1;
> }
>
> - for (i = s->cluster_size; i < offset; i += s->cluster_size)
> + for (i = s->cluster_size; i <= offset; i += s->cluster_size)
> c = modified_fat_get(s, c);
While your change results in the correct behaviour, I think I would
prefer the code to be changed like this so that at the start of each
loop iteration, 'i' always refers to the offset that matches 'c':
for (i = 0; i < offset; i += s->cluster_size) {
c = modified_fat_get(s, c);
}
I'm also adding braces here to make the code conform with the QEMU
coding style. You can use scripts/checkpatch.pl to make sure that all
code you add has the correct style. Much of the vvfat code predates the
coding style, so you'll often have to change style when you touch a
line. (Which is good, because it slowly fixes the existing mess.)
You can keep my Reviewed/Tested-by if you change this.
Kevin
