Re: [PATCH] kill unneeded code from mm/memory.c

[Christoph Hellwig <hch@ns.caldera.de>]

On Wed, Aug 01, 2001 at 09:54:20PM -0400, Pete Zaitcev wrote:
> > diff -uNr ../master/linux-2.4.7-ac3/mm/memory.c linux/mm/memory.c
> > --- ../master/linux-2.4.7-ac3/mm/memory.c	Thu Aug  2 01:48:23 2001
> > +++ linux/mm/memory.c	Thu Aug  2 01:50:12 2001
> > @@ -1041,17 +1041,10 @@
> >  		}
> >  	}
> >  	inode->i_size = offset;
> > -	if (inode->i_op && inode->i_op->truncate)
> > -	{
> > -		/* This doesnt scale but it is meant to be a 2.4 invariant */
> > -		lock_kernel();
> > -		inode->i_op->truncate(inode);
> > -		unlock_kernel();
> > -	}
> > -	return 0;
> > -	
> > +
> >  out_truncate:
> >  	if (inode->i_op && inode->i_op->truncate) {
> > +		/* This doesnt scale but it is meant to be a 2.4 invariant */
> >  		lock_kernel();
> >   		inode->i_op->truncate(inode);
> >  		unlock_kernel();
> 
> I disagree. It is the style to have a function trip exceptions
> by doing goto out_something. Those exceptions are stacked
> in the fall through fashion, but the success case IS NOT.
> By implemention this factorization you save several bytes
> and make just everyone to wonder if there is a bug or mispatch
> with missing "return 0" case here.

Well, out_truncate is a sucessfull exit, too.
Maybe the patch was missing a few lines of context to make the situation
completly clear:

  out_unlock:
  	spin_unlock(&mapping->i_shared_lock);
  	truncate_inode_pages(mapping, offset);
  	goto out_truncate;

  do_expand:
  	if (offset > inode->i_sb->s_maxbytes)
  		goto out;
  	limit = current->rlim[RLIMIT_FSIZE].rlim_cur;
  	if (limit != RLIM_INFINITY) {
  		if (inode->i_size >= limit) {
  			send_sig(SIGXFSZ, current, 0);
  			goto out;
  		}
  		if (offset > limit) {
  			send_sig(SIGXFSZ, current, 0);
  			offset = limit;
  		}
  	}
 	inode->i_size = offset;
-	if (inode->i_op && inode->i_op->truncate)
-	{
-		/* This doesnt scale but it is meant to be a 2.4 invariant */
-		lock_kernel();
-		inode->i_op->truncate(inode);
-		unlock_kernel();
-	}
-	return 0;
-	
+
 out_truncate:
 	if (inode->i_op && inode->i_op->truncate) {
+		/* This doesnt scale but it is meant to be a 2.4 invariant */
 		lock_kernel();
  		inode->i_op->truncate(inode);
 		unlock_kernel();
	}
	return 0;
  out:
	return -EFBIG;
  }