[linux-cifs-client] Re: [PATCH 08/13] cifs: convert cifs_get_inode_info to use cifs_iget

[Jeff Layton]

On Wed, 27 May 2009 09:40:43 -0400
Christoph Hellwig <hch at infradead.org> wrote:

> Looks good.
> 
> 
> Reviewed-by: Christoph Hellwig <hch at lst.de>
> 
> Two little nitpicks:
> 
> On Wed, May 27, 2009 at 08:30:29AM -0400, Jeff Layton wrote:
> >  				access_flags_to_mode(ppace[i]->access_req,
> >  						     ppace[i]->type,
> > -						     &(inode->i_mode),
> > +						     &(fattr->cf_mode),
> >  						     &user_mask);
> >  			if (compare_sids(&(ppace[i]->sid), pgrpsid))
> >  				access_flags_to_mode(ppace[i]->access_req,
> >  						     ppace[i]->type,
> > -						     &(inode->i_mode),
> > +						     &(fattr->cf_mode),
> >  						     &group_mask);
> >  			if (compare_sids(&(ppace[i]->sid), &sid_everyone))
> >  				access_flags_to_mode(ppace[i]->access_req,
> >  						     ppace[i]->type,
> > -						     &(inode->i_mode),
> > +						     &(fattr->cf_mode),
> >  						     &other_mask);
> 
> If you touch these lines please also remove the superflous braces.
> 
> > +	cifs_i->delete_pending = fattr->cf_flags & CIFS_FATTR_DELETE_PENDING;
> > +
> > +	/*
> > +	 * Can't safely change the file size here if the client is writing to
> > +	 * it due to potential races.
> > +	 */
> >  	spin_lock(&inode->i_lock);
> >  	if (is_size_safe_to_change(cifs_i, fattr->cf_eof)) {
> > -		/*
> > -		 * We can not safely change the file size here if the client
> > -		 * is writing to it due to potential races.
> > -		 */
> 
> Why isn't this comment introduced in the correct location in the
> patch adding it?
> 

Thanks for the review. I'll fix the above and will repost the set
soon. I'll also plan to consolidate more of the patches for the next
set to try and keep things bisectable.

-- 
Jeff Layton <jlayton at redhat.com>