On Wed, 2020-04-08 at 14:22 +0300, Dan Carpenter wrote:
> On Wed, Apr 08, 2020 at 02:17:34PM +0300, Dan Carpenter wrote:
> > Hello Jeff Layton,
> >
> > The patch 9a8d03ca2e2c: "ceph: attempt to do async create when
> > possible" from Nov 27, 2019, leads to the following static checker
> > warning:
> >
> > fs/ceph/file.c:540 ceph_async_create_cb()
> > error: uninitialized symbol 'base'.
>
> Sorry, see the unlink function as well.
>
> fs/ceph/dir.c:1072 ceph_async_unlink_cb()
> error: uninitialized symbol 'pathlen'.
>
> regards,
> dan carpenter
>
Many thanks, Dan!
I think the right thing to do is probably to just make
ceph_mdsc_free_path not do anything when it gets a IS_ERR value. At the
same time, this is also an error path, so we might as well initialize
the values we're passing into ceph_mdsc_build_path to make sure the
warnings look sane. See the attached two patches.
Since this code is not upstream yet, I'll probably just move the first
one to be before the async create/delete patches go in, and squash the
second one into the respective patches that add that functionality so we
don't have any regressions.
Sound ok?
--
Jeff Layton <jlayton@xxxxxxxxxx>
From 06e892961013cfbd6f48239794669ba34b812052 Mon Sep 17 00:00:00 2001
From: Jeff Layton <jlayton@xxxxxxxxxx>
Date: Wed, 8 Apr 2020 08:40:33 -0400
Subject: [PATCH 1/2] ceph: have ceph_mdsc_free_path ignore ERR_PTR values
This makes the error handling simpler in some callers.
Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
---
fs/ceph/mds_client.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h
index 1b40f30e0a8e..754e0682398e 100644
--- a/fs/ceph/mds_client.h
+++ b/fs/ceph/mds_client.h
@@ -531,7 +531,7 @@ extern void ceph_mdsc_pre_umount(struct ceph_mds_client *mdsc);
static inline void ceph_mdsc_free_path(char *path, int len)
{
- if (path)
+ if (path && !IS_ERR(path))
__putname(path - (PATH_MAX - 1 - len));
}
--
2.25.2
From 066f69dfd78a20ad9aa5f7e387da6886c8410ce2 Mon Sep 17 00:00:00 2001
From: Jeff Layton <jlayton@xxxxxxxxxx>
Date: Wed, 8 Apr 2020 08:41:38 -0400
Subject: [PATCH 2/2] SQUASH: initialize path and base values in async dirops
cb's
Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
---
fs/ceph/dir.c | 4 ++--
fs/ceph/file.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index 9d02d4feb693..39f5311404b0 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -1057,8 +1057,8 @@ static void ceph_async_unlink_cb(struct ceph_mds_client *mdsc,
/* If op failed, mark everyone involved for errors */
if (result) {
- int pathlen;
- u64 base;
+ int pathlen = 0;
+ u64 base = 0;
char *path = ceph_mdsc_build_path(req->r_dentry, &pathlen,
&base, 0);
diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index 3a1bd13de84f..160644ddaeed 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -529,8 +529,8 @@ static void ceph_async_create_cb(struct ceph_mds_client *mdsc,
if (result) {
struct dentry *dentry = req->r_dentry;
- int pathlen;
- u64 base;
+ int pathlen = 0;
+ u64 base = 0;
char *path = ceph_mdsc_build_path(req->r_dentry, &pathlen,
&base, 0);
--
2.25.2
