kill unneeded code from mm/memory.c

PATCH Pete Zaitcev 2 participants 2 messages Aug 2, 2001 1 message not in the archive

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Pete Zaitcev

author

2001-08-02 01:54 UTC #1 lore raw

Quoted 13 lines

diff -uNr ../master/linux-2.4.7-ac3/mm/memory.c linux/mm/memory.c

mm/memory.c renamed from master/linux-2.4.7-ac3/mm/memory.c +0 -6

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

mm/memory.c line 1050

Quoted 10 lines

mm/memory.c renamed from master/linux-2.4.7-ac3/mm/memory.c +2 -3

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

Christoph Hellwig

2001-08-02 08:41 UTC #2 lore raw

In reply to a message not in the archive

Quoted 22 lines · Pete Zaitcev

Quoted 22 lines

diff -uNr ../master/linux-2.4.7-ac3/mm/memory.c linux/mm/memory.c

mm/memory.c renamed from master/linux-2.4.7-ac3/mm/memory.c +2 -9

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

Quoted 8 lines

mm/memory.c line 1050
Quoted 1 line
mm/memory.c renamed from master/linux-2.4.7-ac3/mm/memory.c +0 -0
 		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;

}