Re: [bug report] fsdax: output address in dax_iomap_pfn() and rename it

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





在 2022/6/7 17:04, Dan Carpenter 写道:
On Tue, Jun 07, 2022 at 04:54:29PM +0800, Shiyang Ruan wrote:


在 2022/6/7 15:59, Dan Carpenter 写道:
Hello Shiyang Ruan,

The patch 1447ac26a964: "fsdax: output address in dax_iomap_pfn() and
rename it" from Jun 3, 2022, leads to the following Smatch static
checker warning:

	fs/dax.c:1085 dax_iomap_direct_access()
	error: uninitialized symbol 'rc'.

fs/dax.c
      1052 static int dax_iomap_direct_access(const struct iomap *iomap, loff_t pos,
      1053                 size_t size, void **kaddr, pfn_t *pfnp)
      1054 {
      1055         pgoff_t pgoff = dax_iomap_pgoff(iomap, pos);
      1056         int id, rc;
      1057         long length;
      1058
      1059         id = dax_read_lock();
      1060         length = dax_direct_access(iomap->dax_dev, pgoff, PHYS_PFN(size),
      1061                                    DAX_ACCESS, kaddr, pfnp);
      1062         if (length < 0) {
      1063                 rc = length;
      1064                 goto out;
      1065         }
      1066         if (!pfnp)
      1067                 goto out_check_addr;

Is this an error path?

      1068         rc = -EINVAL;
      1069         if (PFN_PHYS(length) < size)
      1070                 goto out;
      1071         if (pfn_t_to_pfn(*pfnp) & (PHYS_PFN(size)-1))
      1072                 goto out;
      1073         /* For larger pages we need devmap */
      1074         if (length > 1 && !pfn_t_devmap(*pfnp))
      1075                 goto out;
      1076         rc = 0;
      1077
      1078 out_check_addr:
      1079         if (!kaddr)
      1080                 goto out;

How is it supposed to be handled if both "pfnp" and "kaddr" are NULL?

Smatch says that "kaddr" can never be NULL so this code is just future
proofing but I didn't look at it carefully.

Yes, we always pass the @kaddr in all caller, it won't be NULL now.  And
even @kaddr and @pfnp are both NULL, it won't cause any error.  So, I think
the rc should be initialized to 0 :

  {
         pgoff_t pgoff = dax_iomap_pgoff(iomap, pos);
-       int id, rc;
+       int id, rc = 0;
         long length;

Do I need to fix this and resend a patch to the list?  Or you could help me
fix this?

Could you handle this?  Is this in Andrew's tree?  I think you send a
follow on patch and he'll eventually fold it into the original patch.

OK, got it.


--
Thanks,
Ruan.


regards,
dan carpenter






[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux