From mboxrd@z Thu Jan  1 00:00:00 1970
From: Steven Whitehouse <swhiteho@redhat.com>
Date: Sat, 24 Jan 2015 20:19:47 +0000
Subject: [Cluster-devel] GFS2: Move most of the remaining inode.c into
	ops_inode.c
In-Reply-To: <20150124194503.GA22383@mwanda>
References: <20150124194503.GA22383@mwanda>
Message-ID: <54C3FE63.7040906@redhat.com>
List-Id: <cluster-devel.redhat.com>
To: cluster-devel.redhat.com
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit

Hi,

On 24/01/15 19:45, Dan Carpenter wrote:
> Hello Steven Whitehouse,
>
> The [some really old patch], leads to the following static checker
> warning:
>
> 	fs/gfs2/inode.c:203 gfs2_inode_lookup()
> 	error: passing non negative 13 to ERR_PTR
>
> fs/gfs2/inode.c
>     167                  set_bit(GIF_INVALID, &ip->i_flags);
>     168                  error = gfs2_glock_nq_init(io_gl, LM_ST_SHARED, GL_EXACT, &ip->i_iopen_gh);
>                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> It looks like this function can return GLR_TRYFAILED.  The caller is
> only expecting normal ERR_PTRs so it could cause an oops.
This does not request a try lock, so it should never return the 
GLR_TRYFAILED value. I think the checker is perhaps not following the 
code well enough to figure that out?

Steve.

>     169                  if (unlikely(error))
>     170                          goto fail_iopen;
>     171
>     172                  ip->i_iopen_gh.gh_gl->gl_object = ip;
>     173                  gfs2_glock_put(io_gl);
>     174                  io_gl = NULL;
>     175
>     176                  if (type == DT_UNKNOWN) {
>     177                          /* Inode glock must be locked already */
>     178                          error = gfs2_inode_refresh(GFS2_I(inode));
>     179                          if (error)
>     180                                  goto fail_refresh;
>     181                  } else {
>     182                          inode->i_mode = DT2IF(type);
>     183                  }
>     184
>     185                  gfs2_set_iop(inode);
>     186                  unlock_new_inode(inode);
>     187          }
>     188
>     189          return inode;
>     190
>     191  fail_refresh:
>     192          ip->i_iopen_gh.gh_flags |= GL_NOCACHE;
>     193          ip->i_iopen_gh.gh_gl->gl_object = NULL;
>     194          gfs2_glock_dq_uninit(&ip->i_iopen_gh);
>     195  fail_iopen:
>     196          if (io_gl)
>     197                  gfs2_glock_put(io_gl);
>     198  fail_put:
>     199          ip->i_gl->gl_object = NULL;
>     200          gfs2_glock_put(ip->i_gl);
>     201  fail:
>     202          iget_failed(inode);
>     203          return ERR_PTR(error);
>     204  }
>
> Related:
> fs/gfs2/inode.c:203 gfs2_inode_lookup() error: passing non negative 13 to ERR_PTR
> fs/gfs2/inode.c:218 gfs2_lookup_by_inum() error: passing non negative 13 to ERR_PTR
> fs/gfs2/inode.c:243 gfs2_lookup_by_inum() error: passing non negative 13 to ERR_PTR
> fs/gfs2/inode.c:306 gfs2_lookupi() error: passing non negative 13 to ERR_PTR
> fs/gfs2/inode.c:324 gfs2_lookupi() error: passing non negative 13 to ERR_PTR
> fs/gfs2/inode.c:852 __gfs2_lookup() error: passing non negative 13 to ERR_PTR
> fs/gfs2/inode.c:1567 gfs2_follow_link() error: passing non negative 13 to ERR_PTR
>
> regards,
> dan carpenter