Skip to contentSkip to conversation

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

Pete Zaitcev

author
#1 lore raw
In reply to a message not in the archive
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

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

}