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

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

[Pete Zaitcev]

> 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.

-- Pete

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

[Christoph Hellwig]

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;
  }

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

[Christoph Hellwig]

Hi Alan,

the attached patch kills a one of the two identical conditionals
at the end of vmtruncate.  This piece of code is only in -ac, btw.

Please consider applying.

	Christoph

-- 
Of course it doesn't work. We've performed a software upgrade.


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();