[PATCH U-BOOT v3 25/30] fs: btrfs: Implement btrfs_file_read()

[Qu Wenruo]

On 2020/9/8 上午6:35, Tom Rini wrote:
> On Wed, Jun 24, 2020 at 06:03:11PM +0200, Marek Behún wrote:
> 
>> From: Qu Wenruo <wqu at suse.com>
>>
>> This version of btrfs_file_read() has the following new features:
>> - Tries all mirrors
>> - More handling on unaligned size
>> - Better compressed extent handling
>>   The old implementation doesn't handle compressed extent with offset
>>   properly: we need to read out the whole compressed extent, then
>>   decompress the whole extent, and only then copy the requested part.
>>
>> Signed-off-by: Qu Wenruo <wqu at suse.com>
>> Reviewed-by: Marek Behún <marek.behun at nic.cz>
> 
> Note that this introduces a warning with LLVM-10:
> fs/btrfs/btrfs.c:246:6: warning: variable 'real_size' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
>         if (!len) {
>             ^~~~
> fs/btrfs/btrfs.c:255:12: note: uninitialized use occurs here
>         if (len > real_size - offset)
>                   ^~~~~~~~~
> fs/btrfs/btrfs.c:246:2: note: remove the 'if' if its condition is always true
>         if (!len) {
>         ^~~~~~~~~~
> fs/btrfs/btrfs.c:228:18: note: initialize the variable 'real_size' to silence this warning
>         loff_t real_size;
>                         ^
>                          = 0
> 1 warning generated.
> 
> and I have silenced as suggested.  I'm not 100% happy with that, but
> leave fixing it here and in upstream btrfs-progs to the btrfs-experts.

My bad. The warning is correct, and since the code is U-boot specific,
it doesn't affect kernel (using page) nor btrfs-progs (not really do
file read with offset).

The fix is a little complex.

In fact we need to always call btrfs_size() to grab the real_size, and
then modify @len using real_size, either using real_size directly, or do
some basic calculation.


BTW, I didn't see the btrfs rebuild work merged upstream. Is this
planned or you just grab from some specific branch?

The proper fix would look like this:

diff --git a/fs/btrfs/btrfs.c b/fs/btrfs/btrfs.c
index 786bb4733fbd..a1a7b3cf1afb 100644
--- a/fs/btrfs/btrfs.c
+++ b/fs/btrfs/btrfs.c
@@ -243,14 +243,13 @@ int btrfs_read(const char *file, void *buf, loff_t
offset, loff_t len,
                return -EINVAL;
        }

-       if (!len) {
-               ret = btrfs_size(file, &real_size);
-               if (ret < 0) {
-                       error("Failed to get inode size: %s", file);
-                       return ret;
-               }
-               len = real_size;
+       ret = btrfs_size(file, &real_size);
+       if (ret < 0) {
+               error("Failed to get inode size: %s", file);
+               return ret;
        }
+       if (!len)
+               len = real_size;

        if (len > real_size - offset)
                len = real_size - offset;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200908/63fd36e1/attachment.sig>