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

On Wed, 27 May 2009 09:40:43 -0400
Christoph Hellwig <[email protected]> wrote:

> Looks good.
> 
> 
> Reviewed-by: Christoph Hellwig <[email protected]>
> 
> 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 <[email protected]>
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html