[Ocfs2-devel] [PATCH] ocfs2: clean up some data_ac usage

[Dan Carpenter]

On Tue, Oct 02, 2018 at 03:53:17PM -0700, Andrew Morton wrote:
> On Fri, 7 Sep 2018 15:25:01 +0300 Dan Carpenter <dan.carpenter at oracle.com> wrote:
> 
> > I started looking at this code because of a Smatch false positive:
> > 
> >     fs/ocfs2/move_extents.c:347 ocfs2_defrag_extent()
> >     warn: variable dereferenced before check 'context->data_ac' (see line 304)
> > 
> > context->data_ac can't be NULL so I removed the check.  I changed the
> > next lines to use "data_ac" instead of "context->data_ac" for
> > consistency.  And we don't need to pass "data_ac" to
> > ocfs2_lock_meta_allocator_move_extents() any more so I removed that.
> > 
> > Signed-off-by: Dan Carpenter <dan.carpenter at oracle.com>
> > 
> 
> Please don't forget the ^---$ at end-of-changelog.
> 
> > --- a/fs/ocfs2/move_extents.c
> > +++ b/fs/ocfs2/move_extents.c
> > @@ -160,15 +160,12 @@ static int __ocfs2_move_extent(handle_t *handle,
> >   * lock allocators, and reserving appropriate number of bits for
> >   * meta blocks and data clusters.
> >   *
> > - * in some cases, we don't need to reserve clusters, just let data_ac
> > - * be NULL.
> >   */
> >  static int ocfs2_lock_meta_allocator_move_extents(struct inode *inode,
> >  					struct ocfs2_extent_tree *et,
> >  					u32 clusters_to_move,
> >  					u32 extents_to_split,
> >  					struct ocfs2_alloc_context **meta_ac,
> > -					struct ocfs2_alloc_context **data_ac,
> >  					int extra_blocks,
> >  					int *credits)
> >  {
> > @@ -255,7 +252,6 @@ static int ocfs2_defrag_extent(struct ocfs2_move_extents_context *context,
> >  	ret = ocfs2_lock_meta_allocator_move_extents(inode, &context->et,
> >  						*len, 1,
> >  						&context->meta_ac,
> > -						&context->data_ac,
> >  						extra_blocks, &credits);
> >  	if (ret) {
> >  		mlog_errno(ret);
> > @@ -344,10 +340,10 @@ static int ocfs2_defrag_extent(struct ocfs2_move_extents_context *context,
> >  		mlog_errno(ret);
> >  
> >  out_commit:
> > -	if (need_free && context->data_ac) {
> > +	if (need_free) {
> 
> Current mainline is different here.
> 

Huh...  I wrote it against linux-next, on top of commit 66897d21fa76
("ocfs2: fix clusters leak in ocfs2_defrag_extent()").  But now git says
that patch was applied on Oct 4, two days later after your reply.

Anway, it should apply now again.  Would you like me to resend?

regarsd,
dan carpenter

> >  		struct ocfs2_alloc_context *data_ac = context->data_ac;
> >  
> > -		if (context->data_ac->ac_which == OCFS2_AC_USE_LOCAL)
> > +		if (data_ac->ac_which == OCFS2_AC_USE_LOCAL)
> >  			ocfs2_free_local_alloc_bits(osb, handle, data_ac,
> >  					new_phys_cpos, new_len);
> >  		else
> > @@ -629,7 +625,7 @@ static int ocfs2_move_extent(struct ocfs2_move_extents_context *context,
> >  	ret = ocfs2_lock_meta_allocator_move_extents(inode, &context->et,
> >  						len, 1,
> >  						&context->meta_ac,
> > -						NULL, extra_blocks, &credits);
> > +						extra_blocks, &credits);
> >  	if (ret) {
> >  		mlog_errno(ret);
> >  		goto out;